[FFmpeg-devel] [PATCH] do not set codec tag in raw video encoder

[Baptiste Coudurier]

On 6/10/10 12:09 AM, Stefano Sabatini wrote:
> On date Wednesday 2010-06-09 16:51:58 -0700, Baptiste Coudurier encoded:
>> On 06/09/2010 02:57 PM, Stefano Sabatini wrote:
>>> On date Wednesday 2010-06-09 20:38:15 +0200, Michael Niedermayer encoded:
>>>> On Wed, Jun 09, 2010 at 02:47:21AM -0700, Baptiste Coudurier wrote:
>>>>> On 6/5/10 3:35 PM, Baptiste Coudurier wrote:
>>>>>> Hi
>>>>>>
>>>>>> $subject, let the muxer choose the appropriate one depending on the
>>>>>> format.
>>>>>>
>>>>>>
>>>>>> rawenc_codec_tag.patch
>>>>>>
>>>>>>
>>>>>> Index: libavcodec/rawenc.c
>>>>>> ===================================================================
>>>>>> --- libavcodec/rawenc.c	(revision 23498)
>>>>>> +++ libavcodec/rawenc.c	(working copy)
>>>>>> @@ -35,8 +35,6 @@
>>>>>>        avctx->coded_frame->pict_type = FF_I_TYPE;
>>>>>>        avctx->coded_frame->key_frame = 1;
>>>>>>        avctx->bits_per_coded_sample =
>>>>>> av_get_bits_per_pixel(&av_pix_fmt_descriptors[avctx->pix_fmt]);
>>>>>> -    if(!avctx->codec_tag)
>>>>>> -        avctx->codec_tag = avcodec_pix_fmt_to_codec_tag(avctx->pix_fmt);
>>>>>>        return 0;
>>>>>>    }
>>>>>>
>>>>>
>>>>> Any objection ?
>>>>
>>>> i suspect this might break muxing rawvideo in some containers
>>>> also it does not seem to be completely in line with the api documentation
>>>> in avcodec.h for codec_tag
>>>>
>>>> iam not really objecting to change things but the docs would have to be
>>>> clarified, muxers (especially nut) must be checked so they dont loose
>>>> support for something that worked
>>>
>>> Indeed this change would break NUT rawvideo and the lavfi_pix_fmts
>>> test.
>>
>> Did you check that you didn't break rawvideo in mov before applying
>> your modifications ? The answer is no and it's broken now.
>
> Note that I limited myself to add tags, without to change the logic
> behind, if adding entries broke it then there was already some problem
> with the design. Anyway, what's for sure we need to list the MOV codec
> tags in raw.c up *before* the NUT ones (as MOV tags are also valid NUT
> tags, the reverse is not true), and remove the duplicated codec tags
> entries, that should be enough to fix muxing.

No that won't work. The codec tag must be chosen by the muxer,
not the encoder. No encoder sets the codec tag right now,
why should rawvideo do it ?

This patch is right IMHO, each container supporting rawvideo must do the 
correct relation between pix_fmt <> codec tag

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org