[Ffmpeg-devel] [PATCH] support for VP6 and FLASHSV in the flv muxer

[Benjamin Larsson]

Reimar D?ffinger wrote:

>Hello,
>On Fri, Dec 15, 2006 at 10:53:30PM +0100, Benjamin Larsson wrote:
>  
>
>>+        switch (enc->codec_id) {
>>+            case CODEC_ID_FLV1   :  flags = FLV_CODECID_H263; break;
>>+            case CODEC_ID_FLASHSV:  flags = FLV_CODECID_SCREEN; break;
>>+            case CODEC_ID_VP6F   :  flags = FLV_CODECID_VP6; break;
>>+            case CODEC_ID_VP6    :  flags = FLV_CODECID_VP6; break;
>>    
>>
>
>Why duplicate code? just make the CODEC_ID_VP6F fallthrough to
>CODEC_ID_VP6.
>  
>

Point taken, I forgot one more thing, just before the switch there is a
flags = FLV_CODECID_H263; statement. That one should be removed if I
understood everything correctly.

>  
>
>>+
>>+            default:
>>    
>>
>
>I personally find it ugly to align the ':', and you don't do it for
>"default"...
>
>  
>
I copied the style from flvdec.c. I don't care how it looks like.

>>-    put_be24(pb,size+1); // include flags
>>+    if (enc->codec_id == CODEC_ID_VP6)
>>+        put_be24(pb,size+2);
>>+    else
>>+        put_be24(pb,size+1); // include flags
>>    
>>
>
>Both sizes include the flags, so the comment shouldn't be at only one
>point.
>Not sure if this is more or less ugly:
>put_be24(pb,size+1+(enc->codec_id == CODEC_ID_VP6));
>  
>

I prefer it the way it is, but maybe a comment about the nature of the
hack is in order. (It is for mencoder to ba able to mux vp6 flvs.)

>
>  
>
>>     put_byte(pb,flags);
>>+    if (enc->codec_id == CODEC_ID_VP6)
>>+        put_byte(pb,0);
>>    
>>
>
>Will there always be enough space in the putbyte buffer even with this
>additional byte?
>  
>

No idea.

>Greetings,
>Reimar D?ffinger
>  
>

MvH
Benjamin Larsson

-- 
new tiny signature