framebuffer sizes (was IXUS160/ELPH160 Porting attempt)

reyalp

14079

« on: 22 / December / 2015, 20:20:54 »

Advertisements

Split from http://chdk.setepontos.com/index.php?topic=12321.msg126077#msg126077

Quote from: srsa_4c on 22 / December / 2015, 18:44:42

Not sure if this is what you're looking for, but just in case. This is from the RAM dump I have. Buffer size = 720x360, only the first 240 lines are used.

Yeah, that's the kind of thing I was thinking of.

Quote

edit:
Turns out, these might be necessary (as in the ixus150 port):
Code: [Select]
#undef CAM_BITMAP_HEIGHT #define CAM_BITMAP_HEIGHT 360

I suspect this will cause other problems, although I'm not certain which parts of the code use bitmap height vs assuming it's the same as the CHDK viewport height. PTP live view would be affected, at least. This does explain why Adongy had so much trouble with the bitmap sizes. edit: wrong, see following discussion.

With DRAW_ON_ACTIVE_BITMAP_BUFFER_ONLY, the it shouldn't matter if the buffers aren't one buffer size apart. It seems like making zebra use only the active buffer on these cams shouldn't be a huge problem, and should perform better too.

We could also make get_bitmap_buffer(0|1), which by default would just do vid_get_bitmap_fb or vid_get_bitmap_fb + <width * height>, but could be overridden for these cams.

« Last Edit: 23 / December / 2015, 19:45:25 by reyalp »

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #1 on: 23 / December / 2015, 17:54:16 »

Quote from: reyalp on 22 / December / 2015, 20:20:54

Code: [Select]
#undef CAM_BITMAP_HEIGHT #define CAM_BITMAP_HEIGHT 360I suspect this will cause other problems, although I'm not certain which parts of the code use bitmap height vs assuming it's the same as the CHDK viewport height.

There's also CAM_SCREEN_HEIGHT which is the height of the visible screen area. That is still 240, same as the viewport's height (AFAIK the overlay and the viewports can't have different sizes, not even on DIGIC 6). Does the zebra code not respect that?
Side note: all those the overlay/viewport related definitions and helper functions (vid_get_viewport_height() and so on) make my head spin. They are one of the most confusing parts of a port.

Logged

reyalp

14079

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #2 on: 23 / December / 2015, 18:11:15 »

Quote from: srsa_4c on 23 / December / 2015, 17:54:16

There's also CAM_SCREEN_HEIGHT which is the height of the visible screen area. That is still 240, same as the viewport's height (AFAIK the overlay and the viewports can't have different sizes, not even on DIGIC 6). Does the zebra code not respect that?
Side note: all those the overlay/viewport related definitions and helper functions (vid_get_viewport_height() and so on) make my head spin. They are one of the most confusing parts of a port.

Agreed. In fact, I was probably mixing up CAM_BITMAP_HEIGHT and CAM_SCREEN_HEIGHT in my earlier comment

I'm open to suggestions of how to make it clearer. Probably a topic for 1.5 and a different thread.

Does chdkptp live work correctly when CAM_BITMAP_HEIGHT = 360 on ixus150?

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #3 on: 23 / December / 2015, 18:32:37 »

Quote from: reyalp on 23 / December / 2015, 18:11:15

I'm open to suggestions of how to make it clearer.

Don't know yet. I'd probably like to see something like a big struct with all overlay/viewport related information, but it's easier said than done.

Quote

Probably a topic for 1.5 and a different thread.

Indeed.

Quote

Does chdkptp live work correctly when CAM_BITMAP_HEIGHT = 360 on ixus150?

If you mean the current code in svn, yes, it works (offsets are not implemented, so "special" modes look incorrect, but that's unrelated).

Logged

philmoz

3450

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #4 on: 23 / December / 2015, 18:46:28 »

Quote from: srsa_4c on 23 / December / 2015, 17:54:16

all those the overlay/viewport related definitions and helper functions (vid_get_viewport_height() and so on) make my head spin. They are one of the most confusing parts of a port.

CHDK IQ torture test

The idea was to move camera specific stuff into the port and out of the core code.
I agree its complex; but there are a lot of variations of viewport and bitmap size, visible/hidden areas, offset etc. Some of these also change depending on the camera mode so are not fixed values.

If you can think of a way to simplify it that would be great; but making sure it works with all the combinations is tricky.

Documentation could be better though (not my favourite part)

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

srsa_4c

4451

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #5 on: 23 / December / 2015, 19:30:26 »

Quote from: philmoz on 23 / December / 2015, 18:46:28

The idea was to move camera specific stuff into the port and out of the core code.

There's no problem with that, the CHDK core looks much better than ML's "core" which is riddled with camera-specific ifdefs.

Quote

If you can think of a way to simplify it that would be great; but making sure it works with all the combinations is tricky.

One of my concerns is that I see some possible future additions (pixel format for example) which would further complicate the mess of helper functions. Wouldn't it be possible to make one or more structs for keeping most overlay/viewport related information? Sort of like the existing camera_screen struct.
For example, I found myself using camera_screen values while implementing port-specific functions needed for the experimental sx280 ptp live view support.

Quote

Documentation could be better though

Agree.

@reyalp
it's probably time to split this discussion to the "1.5 plans" thread

Logged

reyalp

14079

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #6 on: 23 / December / 2015, 19:31:10 »

Quote from: srsa_4c on 23 / December / 2015, 18:32:37

If you mean the current code in svn, yes, it works (offsets are not implemented, so "special" modes look incorrect, but that's unrelated).

Yeah, I meant whether the bitmap image lines up correctly with the viewport when "overlay fit" set.

Logged

Don't forget what the H stands for.

reyalp

14079

Re: Re: IXUS160/ELPH160 Porting attempt

« Reply #7 on: 23 / December / 2015, 20:21:44 »

Quote from: srsa_4c on 23 / December / 2015, 19:30:26

@reyalp
it's probably time to split this discussion to the "1.5 plans" thread

I think it's big enough for it's own thread

I agree with Phil quite a bit of the problem because that capturing the behavior of all the different cameras is actually complicated. For example, a lot of the viewport functions need to be functions, because they change at runtime on some cameras.

That said, it's really hard to tell at a glance what values a camera is using, or how those values ended up being used. We currently use values from:

camera.h
platform_camera.h
camera_info structures
generic/wrappers
camera lib.c and/or sub/lib.c

chaining through weak functions makes it even more confusing. E.g. some cameras implicitly use
vid_get_viewport_width_proper() { return vid_get_viewport_width()*2; }
while others might implement _width_proper() make _viewport_width() use half that.

I have always found the camera_info structures hard to follow, because it's very hard to tell which field a particular define goes in. I understand why it is that way (assigning in code would add significant size), but every time I look at those initializations I think there must be a better way.

Using both the camera_info values and the camera.h defines also makes it harder to keep track of, since you have to grep for both, and (because of the initialization thing) you don't see which defines go in which field.

Of course, any significant re-work is going to be difficult since it will require blind changes on a bunch of cams.

Documentation could certainly help.

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: framebuffer sizes (was IXUS160/ELPH160 Porting attempt)

« Reply #8 on: 23 / December / 2015, 20:35:02 »

Quote from: reyalp on 23 / December / 2015, 20:21:44

Of course, any significant re-work is going to be difficult since it will require blind changes on a bunch of cams.

I'd avoid changing all those ports. There could be a default "wrapper" (?) for ports using "legacy" functions.
I'm more worried about the feasibility of a complete re-work: it would be a huge job.

Logged