[FFmpeg-devel] [PATCH] movenc: enable writing of interlace information back to the 'fiel' atom.

[Tim Nicholson]

On 22/10/12 15:07, Michael Niedermayer wrote:
> On Mon, Oct 22, 2012 at 08:08:46AM +0100, Tim Nicholson wrote:
>> On 19/10/12 17:26, Michael Niedermayer wrote:/
>>> [..]
>>>>>>>>      avio_wb16(pb, 1); /* Frame count (= 1) */
>>>>>>>>  
>>>>>>>> +    /* Set AVCodecContext.field_order to match current interlace status
>>>>>>>> +       this is used to control the writing of the "fiel" atom */
>>>>>>>> +    if (track->enc->coded_frame->interlaced_frame)
>>>>>>>> +        track->enc->field_order = track->enc->coded_frame->top_field_first ? AV_FIELD_TB:AV_FIELD_BT;
>>>>>>>
>>>>>>> that looks quite odd, for example assume there is no decoder and
>>>>>>> encoder, but just stream copy, coded_frame will be NULL in that case.
>>>>>>
>>>>>> I originally tested for track->enc->coded_frame but never found an
>>>>>> occasion where it was NULL so took it back out as in the case of a
>>>>>> stream copy the field_order is preserved anyway, forgetting that in this
>>>>>> case it would be NULL, doh! :(
>>>>>>
>>>>>>> or consider the encoder (which can run in a seperate thread) frees
>>>>>>> coded_frame somewhere short before top_field_first is accessed.
>>>>>>
>>>>>> Ahh, your right, I hadn't considered threading, and as I said in my
>>>>>> tests it never happened that way. However this is a more serious issue.
>>>>>> Presumably then there is no way to avoid doing it in the coder rather
>>>>>> than the muxer? I was trying to avoid having to add something to all
>>>>>> relevant coders..
>>>>>
>>>>> the user app could set it before the encoder, but the dox would
>>>>> need to be updated
>>>>>
>>>>
>>>> I was beginning to think that was the most likely common place...
>>>>
>>>> The easy other way I tried was to do it in the encoder_close function
>>>> before any av_freep(&avctx->coded_frame); but then I noticed that for
>>>> rawvideo:-
>>>
>>> have you checked that the encoder inputs interlacing is transported
>>> into coded_frame by most encoders ?
>>>
>>
>> For the ones where it matters, yes. Which is when I found that rawvideo
>> doesn't. The number of codecs affected is actually quite small. The atom
> 
> I was concerned that quite a few of less widespread codecs dont bother
> setting coded_frame.interlaced_frame
> 
> But if it works i surely dont mind, my concern was more
> that someone encoding into a odd game format and storing that in mov
> could end with a incorrect atom in mov saying its progerssive or
> something while it actually is not
>
> another problem i see is that doing it in encoder_close() would
> result in it not being available before encoding is finished.
> this certainly cant work with realtime streaming of any container for
> example
>

Which is why setting field_order directly in *init when all the other
static parameters are set is actually feeling more and more like the
right place to do it. Even if it means tweaking each relevant codec
individually...

> 
>> is mandated for rawvideo (where I still have a problem) and running a
>> selection of transcodes through Quicktime Pro indicate that DV and
>> Prores add it in as well. I could just add a couple of lines into each
>> of the above codecs but thought it worth doing it in a properly
>> extensible way that provided a hook for other codecs as and when it
>> became necessary.
> 
> iam happy with any solution that works ...
> 
> 
> [...]



-- 
Tim