[MPlayer-dev-eng] [PATCH] Color Support for Blinkenlights Output Module

[Stefan Schuermans]

Hello Reimar,

Thanks for reviewing my patch.

Am 29.12.2013 23:29, schrieb Reimar Döffinger:
> On Sun, Dec 29, 2013 at 08:29:44PM +0100, Stefan Schuermans wrote:
>> @@ -306,7 +310,8 @@
>>  
>>  static int query_format(uint32_t format) {
>>  	if (format == bl->img_format)
>> -		return VFCAP_CSP_SUPPORTED|VFCAP_CSP_SUPPORTED_BY_HW;
>> +		return VFCAP_CSP_SUPPORTED | VFCAP_CSP_SUPPORTED_BY_HW |
>> +		       VFCAP_ACCEPT_STRIDE;
>>  	return 0;
> 
> This seems kind of unrelated?

I added VFCAP_ACCEPT_STRIDE to make mplayer call draw_slice also for RGB
format, so I can reuse the code already present in this function.

>> @@ -330,12 +335,12 @@
>>  static int draw_slice(uint8_t *srcimg[], int stride[],
>>  		int w, int h, int x, int y) {
>> [...]
> 
> Hm, I was assuming that draw_slice wouldn't normally be used for RGB.

see above

> (slightly unrelated): Strangely, it seems that many allocations use a multiplication by a
> constant "3" which seems a bit strange.

This was already in the original code from Rik (rev 7327), so I kept it.
But you are right, this was not very clean. I have now changed it to
just allocate the memory needed.

> Same applies to some memset/memcpys. Completely unrelated, I wonder why flip_page introduces a full 1 frame delay,
> and even uses a full memcpy to do so (which is silly either way, double-buffering would make a lot more sense,
> if I could see any sense in the delay in the first place).

I have converted the code to double-buffering.

Double buffering is needed because the file output has to write the
duration first and then the frame data. This is a little bit ugly, but
that's how the file format defined by Blinkenlights works...

> I suspect that a bit of code cleanup wouldn't hurt, even though that probably isn't as much fun as
> watching colourful LEDs ;-)

I tried to do some cleanup.

I also fixed the copyright notice at the top of this file, which looked
like a lot of this code has been written by me. I think it was changed
in rev 27541 - maybe accideltally. Most of the code is still the
original code from Rik, so I added a remark that I just did a few patches.

Best regards,
Stefan

PS: Happy new year!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: MPlayer-svn20140102-blrgb.diff
Type: text/x-patch
Size: 15364 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20140102/9b48270f/attachment.bin>