extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer #7828

theidealist · 2021-09-20T19:33:46Z

It can be helpful when building up native python libraries that utilize external framebuf.Framebuffers to know the width and height of those objects. This PR adds four simple methods to framebuf.Framebuffer that permit retrieval of width, height, format and stride.

Tested locally on the ESP32 GENERIC board port:

>>> w = 400
>>> h = 300
>>> buf = bytearray(w * h // 8)
>>> fb = framebuf.FrameBuffer(buf, w, h, framebuf.MONO_HLSB)
>>> fb.width()
400
>>> fb.height()
300
>>> fb.format()
3
>>> fb.format() == framebuf.MONO_HLSB
True
>>> fb.stride()
400

peterhinch · 2021-09-21T07:24:16Z

I provide access in a subclass. If you are going to do it in the base class I would recommend also providing access to the format. This would aid use of the blit method: to construct a palette you need to know the destination frame buffer's format.

theidealist · 2021-09-21T16:04:53Z

Thanks for the feedback, @peterhinch!

I provide access in a subclass.

Yes, I also do this, but then I wanted to reimplement blit in my subclass (for rotating/inverting) and wanted to maintain the same interface, insofar as I could, anyway, and the blit method accepts just a framebuffer. Maybe this framebuffer is coming from another library and I don't have easy access to it's width/height? It just felt like something the base class should have, I guess.

If you are going to do it in the base class I would recommend also providing access to the format. This would aid use of the blit method: to construct a palette you need to know the destination frame buffer's format.

This is a good suggestion. I'll do this, too.

volkerjaenisch · 2021-09-22T19:16:03Z

Thanks for the feedback, @peterhinch!

I provide access in a subclass.

But what is the point to this? Again memory consumption?

I was right this evening driven to make a subclass of FrameBuffer for testing my new Framebuffer.get_rect(x, y, w, h) functionality. The subclass makes the convoluted test (lots of parameter combinations) even more complicated and tests should be simple.

Also it is IMHO not really Pythonic if an Instance property is write only. In any case it is error prone to have state double accounted. And the double accounting of state becomes not better if it is distributed between two languages.

Used in production a subclass will take IMHO more runtime RAM in the python space than the Data access handlers of @theidealist in the C-Space, where the information (w, h, buf, ..) is already stored.

@theidealist
Can you please send some numers to support or dismiss my assumptions.

@peterhinch
Do the access handlers contributing to the already in C stored data so much RAM of code that it is justified press the users to subclass, any time they like to access data that is already stored? Can you please elaborate on this question as a senior Micropython developer.

cetero censeo : Part1
For RAM critical application, which do not need a framebuffer at all, it is no problem to just disable the framebuffer via

undef MICROPY_PY_FRAMEBUF

and compile a lean micropython, without.
For applications that use a framebuffer IMHO RAM is not so critical. If it were the user would use character displays instead.

cetero censeo : Part2
Let us push framebuf and other modules (the extmod category is named so but sadly is not external) out of the micropython core.
This would have two immediate benefits:

The monolithic micropython would be Pythonically modularized
It would effectively prevent such negative discussions which just stall the further creative development by driving away interested developer that like to contribute

Cheers,
Volker

theidealist · 2021-09-22T21:59:50Z

Thanks for the response, @volkerjaenisch

@theidealist
Can you please send some numers to support or dismiss my assumptions.

I agree with your assessment (if I understand it correctly), but I'm not sure how to provide numbers to support it. Do you mean you'd like to have numbers that show how much RAM a FrameBuffer subclass in Python that stores the width and height in Python uses compared to a FrameBuffer like mine, which just exposes the already-stored data via accessors? I am new to micropython and so am not sure how to get such numbers.

Let us push framebuf and other modules (the extmod category is named so but sadly is not external) out of the micropython core.
This would have two immediate benefits:

The monolithic micropython would be Pythonically modularized

It would effectively prevent such negative discussions which just stall the further creative development by driving away interested developer that like to contribute

I apologize if this has become a "negative discussion" somewhere. I truly did not mean it as such -- I was only trying to contribute to a project I find immensely useful. I, personally, am using the esp32 port on a few devices with various small displays and was, for what it's worth, delighted that FrameBuffer support was provided "out of the box" and that I only had to get into learning how to build and deploy my own firmware once I was ready. I did not intend to stifle anyone's creativity and am happy to have the pull-request denied if it is displeasing to senior developers. I've got my own fork -- I can always do with that whatever I want (open source FTW!).

volkerjaenisch · 2021-09-22T22:14:22Z

@theidealist
Sorry for troubling the waters for you. That was not my intend.

Like you I proposed a simple, reasonable extension to Micropython. Like you I was not greeted with open arms but deflection.

Cheers,
Volker

peterhinch · 2021-09-23T08:28:20Z

I provide access in a subclass.

@volkerjaenisch I was merely stating a fact. Not advocating against doing it in the base class.

I would recommend also providing access to the format.

This surely offers support for doing it in the base class, which I happen to think is a good idea.

volkerjaenisch · 2021-09-24T19:53:23Z

@peterhinch
Thank you for clarification.

@theidealist
I do like your PR quite a lot.

Cheers,
Volker

theidealist · 2021-10-01T14:38:56Z

Is there anything I can or should do to have this PR approved or rejected? Anyone know an appropriate maintainer to reach out to?

Not trying to be pushy, just curious.

theidealist · 2021-10-09T00:37:49Z

@dpgeorge -- I see you've done a lot of the work in the extmod/modframebuf git log. What are your thoughts on this PR?

theidealist · 2021-10-22T18:42:46Z

@peterhinch / @dpgeorge -- do you think I should just abandon this PR or is there a chance it will be reviewed and accepted?

peterhinch · 2021-10-23T07:48:15Z

@theidealist I am not a maintainer, but I think they are busy at the moment. There is quite a queue of pending PR's.

theidealist · 2021-12-02T15:54:25Z

@dpgeorge -- do you think I should just abandon this PR or is there a chance it will be reviewed and accepted?

projectgus · 2024-03-07T23:52:32Z

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

theidealist added 2 commits September 20, 2021 14:38

Add width() and height() to framebuf.Framebuffer

e243e15

Update docs

fe157e5

dpgeorge added the extmod Relates to extmod/ directory in source label Sep 21, 2021

Add accessors for format() and stride() as well

764aefe

theidealist changed the title ~~extmod/modframebuf.c: Expose width and height functions on framebuf.Framebuffer~~ extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer Sep 21, 2021

Fix indexing error in mpy_init

8ab9742

Gadgetoid mentioned this pull request Feb 29, 2024

global: Remove the STATIC macro. #13763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer #7828

extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer #7828

theidealist commented Sep 20, 2021 •

edited

Loading

peterhinch commented Sep 21, 2021

theidealist commented Sep 21, 2021 •

edited

Loading

volkerjaenisch commented Sep 22, 2021

theidealist commented Sep 22, 2021

volkerjaenisch commented Sep 22, 2021 •

edited

Loading

peterhinch commented Sep 23, 2021

volkerjaenisch commented Sep 24, 2021

theidealist commented Oct 1, 2021

theidealist commented Oct 9, 2021

theidealist commented Oct 22, 2021

peterhinch commented Oct 23, 2021

theidealist commented Dec 2, 2021

projectgus commented Mar 7, 2024

extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer #7828

Are you sure you want to change the base?

extmod/modframebuf.c: Expose width(), height(), format(), and stride() accessors on framebuf.Framebuffer #7828

Conversation

theidealist commented Sep 20, 2021 • edited Loading

peterhinch commented Sep 21, 2021

theidealist commented Sep 21, 2021 • edited Loading

volkerjaenisch commented Sep 22, 2021

theidealist commented Sep 22, 2021

volkerjaenisch commented Sep 22, 2021 • edited Loading

peterhinch commented Sep 23, 2021

volkerjaenisch commented Sep 24, 2021

theidealist commented Oct 1, 2021

theidealist commented Oct 9, 2021

theidealist commented Oct 22, 2021

peterhinch commented Oct 23, 2021

theidealist commented Dec 2, 2021

projectgus commented Mar 7, 2024

theidealist commented Sep 20, 2021 •

edited

Loading

theidealist commented Sep 21, 2021 •

edited

Loading

volkerjaenisch commented Sep 22, 2021 •

edited

Loading