[FFmpeg-devel] [PATCH] avformat/mxfenc: support XAVC long gop

[Baptiste Coudurier]

Hey Tomas, I hope you are doing well

> On Mar 29, 2019, at 2:41 AM, Tomas Härdin <tjoppen at acc.umu.se> wrote:
> 
> tor 2019-03-28 klockan 08:50 -0700 skrev Baptiste Coudurier:
>> ---
>>  libavformat/mxf.h    |   1 +
>>  libavformat/mxfenc.c | 194 ++++++++++++++++++++++++++++++++-----------
>>  2 files changed, 145 insertions(+), 50 deletions(-)
>> 
>> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
>> index 4394450dea..f32124f772 100644
>> --- a/libavformat/mxf.h
>> +++ b/libavformat/mxf.h
>> @@ -1317,6 +1330,13 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
>>          avio_w8(pb, sc->field_dominance);
>>      }
>> 
>> +    if (st->codecpar->codec_id == AV_CODEC_ID_H264 && !sc->avc_intra) {
>> +        // write avc sub descriptor ref
>> +        mxf_write_local_tag(pb, 8 + 16, 0x8100);
>> +        mxf_write_refs_count(pb, 1);
>> +        mxf_write_uuid(pb, AVCSubDescriptor, 0);
>> +    }
> 
> Should this always be written for long gop AVC? Not just XAVC? Same
> goes for mxf_write_avc_subdesc() I think, unless I'm reading this incorrectly..

The code will write always write it for AV_CODEC_ID_H264 except if it is avc_intra (it uses the legacy mpegvideo desc),
so it will always be written for long gop as well. There is no check for XAVC technically.

>>  static void mxf_write_s436m_anc_desc(AVFormatContext *s, AVStream *st)
>> @@ -2136,30 +2196,30 @@ static const struct {
>>      int frame_size;
>>      int profile;
>>      uint8_t interlaced;
>> -    int long_gop; // 1 or 0 when there are separate UIDs for Long GOP and Intra, -1 when Intra/LGOP detection can be ignored
>> +    int intra_only; // 1 or 0 when there are separate UIDs for Long GOP and Intra, -1 when Intra/LGOP detection can be ignored
> 
> Any particular reason for inverting this logic? Leaving it as is would
> make the patch smaller..
> 
> /Tomas

Checking long gop looks weird to me since the specs don’t make that distinction, they do however make the explicit distinction for intra only
profile, so it feels more intuitive to mark them intra only. No strong opinion though.

Thanks a lot!

—
Baptiste Coudurier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: Message signed with OpenPGP
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190329/63efacd5/attachment.sig>