[FFmpeg-devel] [PATCH] movenc: enable writing of interlace information back to the 'fiel' atom.
nichot20 at yahoo.com
Fri Oct 19 18:08:33 CEST 2012
On 19/10/12 16:40, Michael Niedermayer wrote:
> On Thu, Oct 18, 2012 at 05:13:49PM +0100, Tim Nicholson wrote:
>> On 18/10/12 16:33, Michael Niedermayer wrote:
>>> On Thu, Oct 18, 2012 at 11:26:13AM +0100, Tim Nicholson wrote:
>>>> Currently interlace information is carried all the way through the
>>>> chain, being modified as required by options such as "-top" and filters
>>>> such as "setfield" but is then ignored when it comes to setting the
>>>> "fiel" atom, which is mandated for some codecs.
>>>> This patch updates the mov style interlace information from ffmpeg's
>>>> internal flags enabling the atom to be written.
>>>> Passes Fate.
>>>> movenc.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>> 5ca2cc166967a4bb7d2dd944cdadfb23378b43c4 0001-movenc-enable-writing-of-interlace-information-back-.patch
>>>> From 49411c0f39fa95c645b630f3fd51a93bcf39a0ce Mon Sep 17 00:00:00 2001
>>>> From: Tim Nicholson <Tim.Nicholson at bbc.co.uk>
>>>> Date: Thu, 18 Oct 2012 11:17:12 +0100
>>>> Subject: [PATCH] movenc: enable writing of interlace information back to the
>>>> 'fiel' atom.
>>>> libavformat/movenc.c | 5 +++++
>>>> 1 files changed, 5 insertions(+), 0 deletions(-)
>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>>> index 0b0bf83..b422b38 100644
>>>> --- a/libavformat/movenc.c
>>>> +++ b/libavformat/movenc.c
>>>> @@ -1103,6 +1103,11 @@ static int mov_write_video_tag(AVIOContext *pb, MOVTrack *track)
>>>> avio_wb32(pb, 0); /* Data size (= 0) */
>>>> 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
a) there is no need to free anything, and
b) coded_frame->interlaced_frame isn't set anyway.
I will ponder some more...
If I were to go with a generalised addition to the close function for
relevant codecs* would a function in avcodec be preffered, or a #define
in avcodec.h as its relatively simple code?
*and treat raw as a special case which it is anyway.
Thanks for your help.
More information about the ffmpeg-devel