[FFmpeg-devel] [PATCH] set/force channelcount in MXF D-10
Michael Niedermayer
michaelni at gmx.at
Thu Jun 26 01:29:18 CEST 2014
On Wed, Jun 25, 2014 at 08:50:38PM +0100, Matthieu Bouron wrote:
> On Wed, Jun 25, 2014 at 11:29 AM, Gaullier Nicolas <
> nicolas.gaullier at arkena.com> wrote:
>
> > >De : ffmpeg-devel-bounces at ffmpeg.org [mailto:
> > ffmpeg-devel-bounces at ffmpeg.org] De la part de Matthieu Bouron
> > >On Tue, Jun 24, 2014 at 6:53 PM, Gaullier Nicolas <
> > nicolas.gaullier at arkena.com> wrote:
> > >> There are interoperability issues with D-10 related to the
> > >> channelcount property in the generic sound essence descriptor.
> > >>
> > >> On one side, SMPTE 386M requires channel count to be 4 or 8, other
> > >> values being prohibited.
> > >> The most widespread value is 8, which seems straightforward as it is
> > >> the actual size of the allocated structure/disk space.
> > >> At the end, it appears that some vendors or workflows do require this
> > >> descriptor to be 8, and otherwise just "fail".
> > >>
> > >> On the other side, at least AVID and ffmpeg do write/set the channel
> > >> count to the exact number of channels really "used", usually 2 or 4,
> > >> or any other value. And on the decoding side, ffmpeg (for
> > >> example) make use of the channel count for probing and only expose
> > >> this limited number of audio streams (which make sense but has strong
> > >> impact on ffmpeg command line usage, output, and downstream workflow).
> > >>
> > >> At the end, I find it pretty usefull to simply give ffmpeg the ability
> > >> to force/set the channel count to any value the user wants.
> > >> (there are turnaround using complex filters, pans, amerge etc., but it
> > >> is quite boring and requires the command line to be adapted to the
> > >> input file
> > >> properties)
> > >>
> > >> -------
> > >> Nicolas
> > >
> > >
> > >Hello Nicolas,
> > >
> > >First of all, thanks for your patch.
> > >
> > >It seems good to me however I would add some check on the channel_count
> > >value:
> > > - to restrict it to 4 or 8 depending on the audio format (since the
> > option is tied to the D10 spec), or at least warn the user.
> > > - warn the user if (channel_count < st->codec->channels) that some
> > stream will be discarded.
> > >
> > >What do you think ?
> >
> > Indeed !
> > I think restriction is a bit risky as (sadly) it appears some
> > vendors/integrators do make use of incompliant channel counts : I had taken
> > the most conservative approach of "warning + inform about this new option".
> > Concerning the case "channel_count < st->codec->channels", I now warn that
> > some channels are discarded, but the processing remains unchanged : the
> > data are actually preserved, and at the end, only the decoder behavior will
> > really make them still available or not.
> > Note: the warnings now only show once when writing the header/preface the
> > first time (I realized that the warnings showed twice because of header
> > metadata refresh when writing the footer).
> >
> > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > index 9572623..6e8ac13 100644
> > --- a/libavformat/mxfenc.c
> > +++ b/libavformat/mxfenc.c
> > @@ -294,6 +294,7 @@ typedef struct MXFContext {
> > uint64_t body_offset;
> > uint32_t instance_number;
> > uint8_t umid[16]; ///< unique material identifier
> > + int channel_count;
> > } MXFContext;
> >
> > static const uint8_t uuid_base[] = {
> > 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd };
> > @@ -996,6 +997,8 @@ static void mxf_write_mpegvideo_desc(AVFormatContext
> > *s, AVStream *st)
> > static void mxf_write_generic_sound_common(AVFormatContext *s, AVStream
> > *st, const UID key, unsigned size)
> > {
> > AVIOContext *pb = s->pb;
> > + MXFContext *mxf = s->priv_data;
> > + int show_warnings = !mxf->footer_partition_offset;
> >
> > mxf_write_generic_desc(s, st, key, size+5+12+8+8);
> >
> > @@ -1009,7 +1012,21 @@ static void
> > mxf_write_generic_sound_common(AVFormatContext *s, AVStream *st, con
> > avio_wb32(pb, 1);
> >
> > mxf_write_local_tag(pb, 4, 0x3D07);
> > - avio_wb32(pb, st->codec->channels);
> > + if (mxf->channel_count == -1) {
> > + if (show_warnings && (s->oformat == &ff_mxf_d10_muxer) &&
> > (st->codec->channels != 4) && (st->codec->channels != 8))
> > + av_log(s, AV_LOG_WARNING, "the number of audio channels shall
> > be 4 or 8 : the output will not comply to MXF D-10 specs, use
> > -mxf_channelcount to fix this\n");
> > + avio_wb32(pb, st->codec->channels);
> > + } else if (s->oformat == &ff_mxf_d10_muxer) {
> > + if (show_warnings && (mxf->channel_count < st->codec->channels))
> > + av_log(s, AV_LOG_WARNING, "mxf_channelcount < actual number
> > of audio channels : some channels will be discarded\n");
> > + if (show_warnings && (mxf->channel_count != 4) &&
> > (mxf->channel_count != 8))
> > + av_log(s, AV_LOG_WARNING, "mxf_channelcount shall be set to 4
> > or 8 : the output will not comply to MXF D-10 specs\n");
> > + avio_wb32(pb, mxf->channel_count);
> > + } else {
> > + if (show_warnings)
> > + av_log(s, AV_LOG_ERROR, "-mxf_channelcount requires MXF D-10
> > and will be ignored\n");
> > + avio_wb32(pb, st->codec->channels);
> > + }
> >
> > mxf_write_local_tag(pb, 4, 0x3D01);
> > avio_wb32(pb, av_get_bits_per_sample(st->codec->codec_id));
> > @@ -2156,6 +2173,19 @@ static int mxf_interleave(AVFormatContext *s,
> > AVPacket *out, AVPacket *pkt, int
> > mxf_interleave_get_packet,
> > mxf_compare_timestamps);
> > }
> >
> > +static const AVOption d10_options[] = {
> > + { "mxf_channelcount", "Force/set channelcount in generic sound
> > essence descriptor",
> > + offsetof(MXFContext, channel_count), AV_OPT_TYPE_INT, {.i64 = -1},
> > -1, 8, AV_OPT_FLAG_ENCODING_PARAM},
> > + { NULL },
> > +};
> > +
> > +static const AVClass mxf_d10_muxer_class = {
> > + .class_name = "MXF-D10 muxer",
> > + .item_name = av_default_item_name,
> > + .option = d10_options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > AVOutputFormat ff_mxf_muxer = {
> > .name = "mxf",
> > .long_name = NULL_IF_CONFIG_SMALL("MXF (Material eXchange
> > Format)"),
> > @@ -2183,4 +2213,5 @@ AVOutputFormat ff_mxf_d10_muxer = {
> > .write_trailer = mxf_write_footer,
> > .flags = AVFMT_NOTIMESTAMPS,
> > .interleave_packet = mxf_interleave,
> > + .priv_class = &mxf_d10_muxer_class,
> > };
> >
>
> The patch looks good to me.
applied
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20140626/7fba02c3/attachment.asc>
More information about the ffmpeg-devel
mailing list