[FFmpeg-devel] [PATCH 1/3] avformat/mxfenc: H.264 Intra support
Michael Niedermayer
michaelni at gmx.at
Tue Sep 16 06:26:22 CEST 2014
On Mon, Sep 15, 2014 at 02:07:06PM +0200, Robert Krüger wrote:
> 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.
no, i refer to, that you work against your own interrest here
that is, i suspect your pushing toward LGPL will lead to the opposit
of what you want
> 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.
Iam not interrested in rewriting code for FLOSS license foo vs.
FLOSS license bar reasons. Nor am i in favor of asking others to do
such things. Though i of course would favor including a LGPL
implementation over a GPL one in ffmpeg as its a feature available
to more useers then ...
but we have no LGPL implementation of AVCI in MXF and noone offering
a bounty to write or relicense one.
The option is just
1. AVCI-MXF under CONFIG_GPL
2. AVCI-MXF under CONFIG_GPL in a seperate branch (yeah i probably
need it so it will be somewhere)
3. someone paying or convincing baptiste to relicese it to LGPL
4. someone paying or convincing someone to rewrite it under LGPL
iam tired, maybe i forgot some option, but these seem the obvious
ones
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140916/6bb5445e/attachment.asc>
More information about the ffmpeg-devel
mailing list