[FFmpeg-devel] [PATCH 1/3] avformat/mxfenc: H.264 Intra support
Robert Krüger
krueger at lesspain.de
Mon Sep 15 14:07:06 CEST 2014
On Mon, Sep 15, 2014 at 1:12 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Sep 15, 2014 at 10:26:28AM +0200, Robert Krüger wrote:
>> On Sun, Sep 14, 2014 at 5:58 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Sun, Sep 14, 2014 at 05:40:35PM +0200, Robert Krüger wrote:
>> >> On Sat, Sep 13, 2014 at 12:36 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > From: Baptiste Coudurier <baptiste.coudurier at gmail.com>
>> >> >
>> >> > Ported by michael from ffmbc to ffmpeg
>> >> > the code is under CONFIG_GPL as ffmbc is GPL
>> >> >
>> >> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> >> > ---
>> >> > libavformat/mxfenc.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> > 1 file changed, 143 insertions(+)
>> >> >
>> >> > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
>> >> > index 6a6b7c2..98f3479 100644
>> >> > --- a/libavformat/mxfenc.c
>> >> > +++ b/libavformat/mxfenc.c
>> >> > @@ -40,6 +40,8 @@
>> >> > #include "libavutil/avassert.h"
>> >> > #include "libavcodec/bytestream.h"
>> >> > #include "libavcodec/dnxhddata.h"
>> >> > +#include "libavcodec/h264.h"
>> >> > +#include "libavcodec/internal.h"
>> >> > #include "audiointerleave.h"
>> >> > #include "avformat.h"
>> >> > #include "avio_internal.h"
>> >> > @@ -95,6 +97,9 @@ static const struct {
>> >> > { AV_CODEC_ID_PCM_S16LE, 1 },
>> >> > { AV_CODEC_ID_DVVIDEO, 15 },
>> >> > { AV_CODEC_ID_DNXHD, 24 },
>> >> > +#if CONFIG_GPL
>> >> > + { AV_CODEC_ID_H264, 34 },
>> >> > +#endif
>> >> > { AV_CODEC_ID_NONE }
>> >> > };
>> >> >
>> >> > @@ -266,6 +271,13 @@ static const MXFContainerEssenceEntry mxf_essence_container_uls[] = {
>> >> > { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
>> >> > { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x13,0x00,0x00 },
>> >> > mxf_write_cdci_desc },
>> >> > +#if CONFIG_GPL
>> >> > + // H.264
>> >> > + { { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x02,0x0D,0x01,0x03,0x01,0x02,0x10,0x60,0x01 },
>> >> > + { 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0x05,0x00 },
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x00,0x00,0x00 },
>> >> > + mxf_write_mpegvideo_desc },
>> >> > +#endif // CONFIG_GPL
>> >> > { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
>> >> > { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
>> >> > { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },
>> >> > @@ -1564,6 +1576,130 @@ static int mxf_parse_dv_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt)
>> >> > return 1;
>> >> > }
>> >> >
>> >> > +#if CONFIG_GPL
>> >> > +static const UID mxf_h264_codec_uls[] = {
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x30,0x00,0x00 }, // AVC Video
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x00,0x00 }, // AVC Intra
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x20,0x00 }, // AVC High 10 Intra
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x01 }, // AVC High 10 Intra RP2027 Class 50 1080/59.94i
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x02 }, // AVC High 10 Intra RP2027 Class 50 1080/50i
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x03 }, // AVC High 10 Intra RP2027 Class 50 1080/29.97p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x04 }, // AVC High 10 Intra RP2027 Class 50 1080/25p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x08 }, // AVC High 10 Intra RP2027 Class 50 720/59.94p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x21,0x09 }, // AVC High 10 Intra RP2027 Class 50 720/50p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x30,0x00 }, // AVC High 422 Intra
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x01 }, // AVC High 422 Intra RP2027 Class 100 1080/59.94i
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x02 }, // AVC High 422 Intra RP2027 Class 100 1080/50i
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x03 }, // AVC High 422 Intra RP2027 Class 100 1080/29.97p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x04 }, // AVC High 422 Intra RP2027 Class 100 1080/25p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x08 }, // AVC High 422 Intra RP2027 Class 100 720/59.94p
>> >> > + { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x32,0x31,0x09 }, // AVC High 422 Intra RP2027 Class 100 720/50p
>> >> > +};
>> >> > +
>> >> > +static const UID *mxf_get_h264_codec_ul(AVCodecContext *avctx, SPS *sps)
>> >> > +{
>> >> > + int long_gop = avctx->gop_size > 1 || avctx->has_b_frames;
>> >> > +
>> >> > + if ((sps->constraint_set_flags >> 3) & 1) {
>> >> > + if (sps->profile_idc == 110)
>> >> > + return &mxf_h264_codec_uls[2];
>> >> > + else if (sps->profile_idc == 122)
>> >> > + return &mxf_h264_codec_uls[9];
>> >> > + }
>> >> > + if (long_gop)
>> >> > + return &mxf_h264_codec_uls[0];
>> >> > + else
>> >> > + return &mxf_h264_codec_uls[1];
>> >> > +}
>> >> > +
>> >> > +static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st,
>> >> > + AVPacket *pkt, MXFIndexEntry *e)
>> >> > +{
>> >> > + MXFStreamContext *sc = st->priv_data;
>> >> > + H264Context h;
>> >> > + const uint8_t *buf = pkt->data;
>> >> > + const uint8_t *buf_end = pkt->data + pkt->size;
>> >> > + uint32_t state = -1;
>> >> > + AVRational sar = {1, 1};
>> >> > +
>> >> > + if (pkt->size < 5 || AV_RB32(pkt->data) != 0x0000001) {
>> >> > + av_log(s, AV_LOG_ERROR, "h264 bitstream malformated, "
>> >> > + "no startcode found, use -vbsf h264_mp4toannexb\n");
>> >> > + return 0;
>> >> > + }
>> >> > +
>> >> > + memset(&h, 0, sizeof(h));
>> >> > + h.avctx = st->codec;
>> >> > + h.thread_context[0] = &h;
>> >> > + h.prev_frame_num = -1;
>> >> > +
>> >> > + for (;;) {
>> >> > + int src_length, dst_length, consumed;
>> >> > + const uint8_t *ptr;
>> >> > +
>> >> > + buf = avpriv_find_start_code(buf, buf_end, &state);
>> >> > + if (buf >= buf_end)
>> >> > + break;
>> >> > + --buf;
>> >> > + src_length = buf_end - buf;
>> >> > + switch (state & 0x1f) {
>> >> > + case NAL_SLICE:
>> >> > + case NAL_IDR_SLICE:
>> >> > + // Do not walk the whole buffer just to decode slice header
>> >> > + if (src_length > 20)
>> >> > + src_length = 20;
>> >> > + break;
>> >> > + }
>> >> > +
>> >> > + ptr = ff_h264_decode_nal(&h, buf, &dst_length, &consumed, src_length);
>> >> > + if (!ptr || dst_length < 0)
>> >> > + break;
>> >> > +
>> >> > + init_get_bits(&h.gb, ptr, 8*dst_length);
>> >> > + //av_log(avctx, AV_LOG_DEBUG, "nal_unit_type:%d\n", h.nal_unit_type);
>> >> > + switch (h.nal_unit_type) {
>> >> > + case NAL_SPS:
>> >> > + ff_h264_decode_seq_parameter_set(&h);
>> >> > + if (h.sps.sar.num > 0 && h.sps.sar.den > 0) {
>> >> > + sar.num = st->codec->width * h.sps.sar.num;
>> >> > + sar.den = st->codec->height * h.sps.sar.den;
>> >> > + }
>> >> > + sc->aspect_ratio.num = st->codec->width * sar.num;
>> >> > + sc->aspect_ratio.den = st->codec->height * sar.den;
>> >> > + av_reduce(&sc->aspect_ratio.num, &sc->aspect_ratio.den,
>> >> > + sar.num, sar.den, 1024*1024);
>> >> > +
>> >> > + sc->interlaced = !h.sps.frame_mbs_only_flag;
>> >> > + sc->component_depth = h.sps.bit_depth_luma;
>> >> > +
>> >> > + sc->codec_ul = mxf_get_h264_codec_ul(st->codec, &h.sps);
>> >> > + e->flags |= 0x40;
>> >> > + break;
>> >> > + case NAL_PPS:
>> >> > + ff_h264_decode_picture_parameter_set(&h, h.gb.size_in_bits);
>> >> > + if (h.sps.timing_info_present_flag) {
>> >> > + if (st->codec->time_base.num != h.sps.num_units_in_tick ||
>> >> > + st->codec->time_base.den*2 != h.sps.time_scale) {
>> >> > + av_log(s, AV_LOG_ERROR, "framerate does not match bitstream values: %d/%d != %d/%d\n",
>> >> > + h.sps.num_units_in_tick, h.sps.time_scale, st->codec->time_base.num, st->codec->time_base.den*2);
>> >> > + return 0;
>> >> > + }
>> >> > + }
>> >> > + if (e->flags & 0x40) // sequence header present
>> >> > + e->flags |= 0x80; // random access
>> >> > + break;
>> >> > + case NAL_IDR_SLICE:
>> >> > + break;
>> >> > + case NAL_SLICE:
>> >> > + av_log(s, AV_LOG_ERROR, "mxf muxer only supports AVC Intra currently\n");
>> >> > + return 0;
>> >> > + }
>> >> > + }
>> >> > +
>> >> > + return !!sc->codec_ul;
>> >> > +}
>> >> > +#endif // CONFIG_GPL
>> >> > +
>> >> > static const UID mxf_mpeg2_codec_uls[] = {
>> >> > { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x01,0x10,0x00 }, // MP-ML I-Frame
>> >> > { 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x03,0x04,0x01,0x02,0x02,0x01,0x01,0x11,0x00 }, // MP-ML Long GOP
>> >> > @@ -1969,6 +2105,13 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket *pkt)
>> >> > av_log(s, AV_LOG_ERROR, "could not get dv profile\n");
>> >> > return -1;
>> >> > }
>> >> > +#if CONFIG_GPL
>> >> > + } else if (st->codec->codec_id == AV_CODEC_ID_H264) {
>> >> > + if (!mxf_parse_h264_frame(s, st, pkt, &ie)) {
>> >> > + av_log(s, AV_LOG_ERROR, "could not get h264 profile and level\n");
>> >> > + return -1;
>> >> > + }
>> >> > +#endif // CONFIG_GPL
>> >> > }
>> >> >
>> >> > if (!mxf->header_written) {
>> >> > --
>> >> > 1.7.9.5
>> >> >
>> >>
>> >> Just a general policy-question: will changes like this not in a way
>> >> discourage development of these features in ffmpeg under LGPL? If
>> >> someone now uses this code (e.g. mxf_parse_h264_frame) in other code
>> >> they will have to make that GPL as well. In the long run this will
>> >> make life harder for library users who cannot live with GPL. So far
>> >> this hasn't been done on that level, at least not in the part of code
>> >> that I am watching. AFAIR currently license borders are normally on a
>> >> component level, i.e. a certain codec, format, filter only works only
>> >> under GPL, not parts of functionality of a cocec, filter or muxer,
>> >> which I think makes it OK to track by people for whom license matters.
>> >> this extends this to a level where one would have to know which
>> >> features of a certain muxer, codec, filter works under which license.
>> >> Of course that's a project maintainer decision. Just wanted to throw
>> >> in that side of the story, so it can be consciously taken into account
>> >> or disregarded.
>> >
>> > This isnt the first "#if CONFIG_GPL" in the code.
>>
>> Technically correct but I see only me_cmp.c and flac_dsp_init.c so far
>> and I don't know what the code does better with GPL. So far I have not
>> run into this as a library user and thus was not aware of it. At least
>> it seems to be a rare exception. I am afraid of the situation it may
>> create, if picking stuff from ffmbc and introducing it via #if
>> CONFIG_GPL becomes more common.
>
> I really think you are doing a "own goal" here
I am not sure I understand what you mean by this. I am not trying to
hide what's my interest in this, if that's what you're referring to.
Also assuming this I am not alone in this situation, so I am bringing
this to your attention to consider as project maintainer, one of whose
goals might be to make adoption of ffmpeg in commercial
projects/software easier. I may be wrong but I interpreted vlc's move
to relicense as much code as possible to LGPL to be motivated by that,
potentially because they thought, the project benefits from this. I
know there is the justified impression that commercial users don't
give back enough but I am convinced that at least tons of bug reports
and samples come from people who use ffmpeg to build commercial
software. Again, this is a decision the project needs to make. I am
just pointing out that intermixing more GPL code in core places makes
life harder for those people, which, of course, includes us. That's
why I don't think this patch is a minor thing.
> do you prefer if i do this work in a seperate branch outside FFmpeg
> or what do you suggest ?
No, the last thing anyone needs is another fork.
I just thought I remembered from past discussions, that this being
consciously avoided (I would have to look that up in the archives but
Stefano's reaction in
http://mplayerhq.hu/pipermail/ffmpeg-devel-irc/2013-December/001805.html
somewhat implies that just as a first example). After all there's
probably lots of useful GPL code in other projects like AVISynth that
would help in some places and opening the door to have that come into
core places of ffmpeg code (I am not talking about a well-defined
component like a new filter) would create a situation that's
definitely a step back for LGPL users at least in the long run.
In this particular case, maybe asking for a bounty for adding AVCI
support in the mxf muxer could be a way. But since this is more a
general question about project policy, maybe it should be defined, if
it is not defined yet. Of course, this might mean that a certain
feature, only available in GPL code in another project like ffmbc,
could not move into some places of ffmpeg code, if the project
maintainers/devs decide that this kind of LGPL user base is important
enough to them.
More information about the ffmpeg-devel
mailing list