2010/2/14 Reimar D?ffinger <Reimar.Doeffinger at gmx.de>:
> On Sun, Feb 14, 2010 at 03:08:02PM +0100, Giorgio wrote:
>> I made all the corrections, I hope it's better now. This is my first
>> patch, so thank you for your patience.
>
> I have some more comments, I can of course easily make the modifications
> myself, but I thought maybe you're interested in the comments anyway.
Of course I'm interested in the comments :)
>> +/* This function converts Theora pixelformat to the corresponding IMGFMT_ */
>> +static unsigned int convert_pixelformat(sh_video_t *sh){
>
> I'd change it to
> static uint32_t theora_pixelformat2imgfmt(theora_pixelformat fmt)
> (so the argument is then context->inf.pixelformat)
I agree, better name (more clear), better return type (more portable),
better argument.
Fixed.
>> + ? ?switch(context->inf.pixelformat) {
>> + ? ? ? case OC_PF_420: return IMGFMT_YV12;
>> + ? ? ? case OC_PF_422: return IMGFMT_422P;
>> + ? ? ? case OC_PF_444: return IMGFMT_444P;
>> + ? ?}
>
> Misses a return for the case that none of these match.
> I'd suggest adding a return 0;
Fixed.
What do you think now?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer_theora_422_444_3.diff
Type: text/x-diff
Size: 2023 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-users/attachments/20100214/b778b44c/attachment-0001.diff>
