Re: [FFmpeg-devel] [PATCH v8 0/4] Fix mpeg1/2 stream copy
> De : Nicolas Gaullier > Envoyé : jeudi 5 mars 2020 09:17 > À : ffmpeg-devel@ffmpeg.org > Cc : Gaullier Nicolas > Objet : [PATCH v8 0/4] Fix mpeg1/2 stream copy > > I have not received any feedback yet. > I repost here with all 4 patches, maybe it is easier for the review > - 1/4: fix mpeg12 decoder use of avctx->rc_buffer_size with is forbidden by > API doc > - 2/4,3/4,4/4: fix mpeg1/2 stream copy > Thank you > > Nicolas Gaullier (4): > avcodec/mpeg12dec: Do not alter avctx->rc_buffer_size > avformat/utils: Make find_stream_info get side data from codec context > avcodec/utils: Fix ff_add_cpb_side_data() add twice > avcodec/mpeg12dec: Add CPB coded side data > > libavcodec/mpeg12dec.c | 16 > libavcodec/utils.c | 5 + > libavformat/utils.c | 18 ++ > tests/ref/fate/mxf-probe-d10 | 3 +++ > tests/ref/fate/ts-demux | 2 +- > 5 files changed, 39 insertions(+), 5 deletions(-) > > -- > 2.25.0.windows.1 ping ? https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=464 Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: Do not alter avctx->rc_buffer_size
> De : ffmpeg-devel De la part de Hendrik > Leppkes > Envoyé : lundi 24 février 2020 17:26 > À : FFmpeg development discussions and patches > Objet : Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: Do not alter > avctx->rc_buffer_size > > On Mon, Feb 24, 2020 at 5:13 PM Nicolas Gaullier > wrote: > > > > rc_buffer_size doesn't really have a meaning to a decoder (its for > encoders and muxers), so why should it not be able to change it to > match the value it reads from the bitstream? > > - Hendrik Both because it is not helpful : a later patch will make the value available as side data, and because API doc (avcodec.h) says "decoding: unused". The other way would have been to update the doc etc., but it does not seem relevant here. This comes from a previous review by James and I agreed with him not to change the API. As soon as this patch is applied, I will send my updated v4 patchset "Fix mpeg1/2 stream copy" which will allow reading rc_buffer_size through side data. Sorry about that, maybe I should have send all of my patches in a bunch, but this little patch is somewhat unrelated to my original patchset and I thought it was better to separate things this way. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data
> De : ffmpeg-devel De la part de Michael
> Niedermayer
> Envoyé : vendredi 21 février 2020 12:54
> À : FFmpeg development discussions and patches
> Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded
> side data
>
> On Thu, Feb 20, 2020 at 11:22:33AM +, Gaullier Nicolas wrote:
> > > De : ffmpeg-devel De la part de Michael
> > > Niedermayer
> > > Envoyé : vendredi 7 février 2020 23:39
> > > À : FFmpeg development discussions and patches
> > > Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB
> > > coded side data
> > >
> > > On Wed, Jan 15, 2020 at 12:42:13AM +0100, Nicolas Gaullier wrote:
> > > > This fixes mpeg2video stream copies to mpeg muxer like this:
> > > > ffmpeg -i xdcamhd.mxf -c:v copy output.mpg
> > > > ---
> > > > libavcodec/mpeg12dec.c | 7 +++
> > > > tests/ref/fate/mxf-probe-d10 | 3 +++
> > > > tests/ref/fate/ts-demux | 2 +-
> > > > 3 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index
> > > > 17f9495a1d..48ac14fafa 100644
> > > > --- a/libavcodec/mpeg12dec.c
> > > > +++ b/libavcodec/mpeg12dec.c
> > > > @@ -1398,6 +1398,7 @@ static void
> > > > mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > > > MpegEncContext *s = >mpeg_enc_ctx;
> > > > int horiz_size_ext, vert_size_ext;
> > > > int bit_rate_ext;
> > > > +AVCPBProperties *cpb_props;
> > > >
> > > > skip_bits(>gb, 1); /* profile and level esc*/
> > > > s->avctx->profile = get_bits(>gb, 3);
> > > > @@ -1429,6 +1430,12 @@ static void
> > > > mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > > > ff_dlog(s->avctx, "sequence extension\n");
> > > > s->codec_id = s->avctx->codec_id = AV_CODEC_ID_MPEG2VIDEO;
> > > >
> > > > +if (cpb_props = ff_add_cpb_side_data(s->avctx)) {
> > > > +cpb_props->buffer_size = FFMAX(cpb_props->buffer_size,
> > > > s->avctx->rc_buffer_size);
> > > > +if (s->bit_rate != 0x3*400)
> > > > +cpb_props->max_bitrate = FFMAX(cpb_props->max_bitrate,
> > > > s->bit_rate);
> > > > +}
> > >
> > > why does this not export exactly the numbers as read from the header?
> > >
> > > thx
> > The header values are expressed in units of 400bit/s, and the native value
> > 0x3 is reserved, in case of MPEG-1 (but the code is
> shared), for vbr signalling.
> > This is not very nice to read, but this is how it is implemented in current
> > code.
>
> you misunderstand, why do you take the maximum of several things instead of
> exporting the value from the header ?
>
> Thanks
Sorry for my misunderstanding. I thought the cpb properties had to reflect the
entire stream at the end and thus cumulate the size/max values.
I agree it is best to have an exact match with native header values.
Thank you for your feedback.
I will send a new version with the 2x FFMAX removed.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data
> De : ffmpeg-devel De la part de James Almer
> Envoyé : vendredi 7 février 2020 23:48
> À : ffmpeg-devel@ffmpeg.org
> Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded
> side data
>
> On 2/7/2020 7:43 PM, James Almer wrote:
> > On 1/14/2020 8:42 PM, Nicolas Gaullier wrote:
> >> This fixes mpeg2video stream copies to mpeg muxer like this:
> >> ffmpeg -i xdcamhd.mxf -c:v copy output.mpg
> >> ---
> >> libavcodec/mpeg12dec.c | 7 +++
> >> tests/ref/fate/mxf-probe-d10 | 3 +++
> >> tests/ref/fate/ts-demux | 2 +-
> >> 3 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> >> index 17f9495a1d..48ac14fafa 100644
> >> --- a/libavcodec/mpeg12dec.c
> >> +++ b/libavcodec/mpeg12dec.c
> >> @@ -1398,6 +1398,7 @@ static void
> >> mpeg_decode_sequence_extension(Mpeg1Context *s1)
> >> MpegEncContext *s = >mpeg_enc_ctx;
> >> int horiz_size_ext, vert_size_ext;
> >> int bit_rate_ext;
> >> +AVCPBProperties *cpb_props;
> >>
> >> skip_bits(>gb, 1); /* profile and level esc*/
> >> s->avctx->profile = get_bits(>gb, 3);
> >> @@ -1429,6 +1430,12 @@ static void
> >> mpeg_decode_sequence_extension(Mpeg1Context *s1)
> >> ff_dlog(s->avctx, "sequence extension\n");
> >> s->codec_id = s->avctx->codec_id = AV_CODEC_ID_MPEG2VIDEO;
> >>
> >> +if (cpb_props = ff_add_cpb_side_data(s->avctx)) {
> >> +cpb_props->buffer_size = FFMAX(cpb_props->buffer_size,
> >> s->avctx->rc_buffer_size);
> >
> > cpb_props->buffer_size is initialized as zero, and rc_buffer_size is not
> > meant to be used for decoding, so i imagine is also zero.
>
> Ok, so i see this decoder is setting rc_buffer_size despite the doxy
> stating it shouldn't, so nevermind that part.
>
> I think this should be done using an internal field in Mpeg1Context
> instead, or the API changed and it reflected in the doxy. I'm more
> inclined for the former option, since you'll be exporting the value
> using AVCPBProperties after all.
I agree with you, but this is not related to my patchset, so this should be
fixed in another patch.
And I would like not to postpone this again... do you mind if this patch is
applied first and if I fix this mpeg12dec design issue later (I am committing
myself to do it) ?
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data
> De : ffmpeg-devel De la part de Michael
> Niedermayer
> Envoyé : vendredi 7 février 2020 23:39
> À : FFmpeg development discussions and patches
> Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded
> side data
>
> On Wed, Jan 15, 2020 at 12:42:13AM +0100, Nicolas Gaullier wrote:
> > This fixes mpeg2video stream copies to mpeg muxer like this:
> > ffmpeg -i xdcamhd.mxf -c:v copy output.mpg
> > ---
> > libavcodec/mpeg12dec.c | 7 +++
> > tests/ref/fate/mxf-probe-d10 | 3 +++
> > tests/ref/fate/ts-demux | 2 +-
> > 3 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index
> > 17f9495a1d..48ac14fafa 100644
> > --- a/libavcodec/mpeg12dec.c
> > +++ b/libavcodec/mpeg12dec.c
> > @@ -1398,6 +1398,7 @@ static void
> > mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > MpegEncContext *s = >mpeg_enc_ctx;
> > int horiz_size_ext, vert_size_ext;
> > int bit_rate_ext;
> > +AVCPBProperties *cpb_props;
> >
> > skip_bits(>gb, 1); /* profile and level esc*/
> > s->avctx->profile = get_bits(>gb, 3);
> > @@ -1429,6 +1430,12 @@ static void
> > mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > ff_dlog(s->avctx, "sequence extension\n");
> > s->codec_id = s->avctx->codec_id = AV_CODEC_ID_MPEG2VIDEO;
> >
> > +if (cpb_props = ff_add_cpb_side_data(s->avctx)) {
> > +cpb_props->buffer_size = FFMAX(cpb_props->buffer_size,
> > s->avctx->rc_buffer_size);
> > +if (s->bit_rate != 0x3*400)
> > +cpb_props->max_bitrate = FFMAX(cpb_props->max_bitrate,
> > s->bit_rate);
> > +}
>
> why does this not export exactly the numbers as read from the header?
>
> thx
The header values are expressed in units of 400bit/s, and the native value
0x3 is reserved, in case of MPEG-1 (but the code is shared), for vbr
signalling.
This is not very nice to read, but this is how it is implemented in current
code.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 0/3] Fix mpeg1/2 stream copy
>Envoyé : vendredi 31 janvier 2020 09:59 >Objet : Re: [FFmpeg-devel] [PATCH v7 0/3] Fix mpeg1/2 stream copy > >I have not received any feedback yet on this latest version that does not >affect the public API. >The 3 patches are still available in patchwork: >https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-2-nicolas.gaullier@cji.paris/ >https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-3-nicolas.gaullier@cji.paris/ >https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-4-nicolas.gaullier@cji.paris/ > >I think it should be near to be approved, could someone take some time for >this review ? Thank you. Ping? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 0/3] Fix mpeg1/2 stream copy
>Envoyé : mercredi 15 janvier 2020 00:42 >À : ffmpeg-devel@ffmpeg.org >Objet : [PATCH v7 0/3] Fix mpeg1/2 stream copy > >Modified with Anton feedback: no public amendment, the code from >add_coded_side_data() is now duplicated from existing one in ffmpeg.c, but it >is rather small. > >Nicolas Gaullier (3): > avformat/utils: Make find_stream_info get side data from codec context > avcodec/utils: Fix ff_add_cpb_side_data() add twice > avcodec/mpeg12dec: Add CPB coded side data > > libavcodec/mpeg12dec.c | 7 +++ > libavcodec/utils.c | 5 + > libavformat/utils.c | 18 ++ > tests/ref/fate/mxf-probe-d10 | 3 +++ > tests/ref/fate/ts-demux | 2 +- > 5 files changed, 34 insertions(+), 1 deletion(-) I have not received any feedback yet on this latest version that does not affect the public API. The 3 patches are still available in patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-2-nicolas.gaullier@cji.paris/ https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-3-nicolas.gaullier@cji.paris/ https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200114234213.3224-4-nicolas.gaullier@cji.paris/ I think it should be near to be approved, could someone take some time for this review ? Thank you. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 5/8] avformat/wavdec: s337m support
>> >> Add s337m probing/reading similarly to spdif.
>> >> ---
>> >> libavformat/wavdec.c | 23 +++
>> >> 1 file changed, 19 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
>> >> 575c667452..d030ed9f9d 100644
>> >> --- a/libavformat/wavdec.c
>> >> +++ b/libavformat/wavdec.c
>> >> @@ -41,6 +41,7 @@
>> >> #include "riff.h"
>> >> #include "w64.h"
>> >> #include "spdif.h"
>> >> +#include "s337m.h"
>> >>
>> >> typedef struct WAVDemuxContext {
>> >> const AVClass *class;
>> >> @@ -55,15 +56,17 @@ typedef struct WAVDemuxContext {
>> >> int audio_eof;
>> >> int ignore_length;
>> >> int spdif;
>> >> +int s337m;
>> >> int smv_cur_pt;
>> >> int smv_given_first;
>> >> int unaligned; // e.g. if an odd number of bytes ID3 tag was
>> >> prepended
>> >> int rifx; // RIFX: integer byte order for parameters is big
>> >> endian } WAVDemuxContext;
>> >>
>> >> -static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav)
>> >> +static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext
>> >> +*wav)
>> >> {
>> >> -if (CONFIG_SPDIF_DEMUXER && s->streams[0]->codecpar->codec_tag == 1)
>> >> {
>> >> +AVCodecParameters *par = s->streams[0]->codecpar;
>> >> +if ((CONFIG_SPDIF_DEMUXER || CONFIG_S337M_DEMUXER) &&
>> >> + par->codec_tag == 1) {
>> >
>> >Did you test this with both "--disable-everything --enable-demuxer=spdif"
>> >and "--disable-everything --enable-demuxer=s337m"?
>> >Neither should fail compilation.
>> >
>> >Carl Eugen
>>
>> I just have tested both, and compilation is ok.
>
>Sorry, this should have been "--disable-everything --enable-demuxer=wav,spdif"
>and "--disable-everything --enable-demuxer=wav,s337m".
>
>Carl Eugen
Sorry, I missed that. I send a new patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 5/8] avformat/wavdec: s337m support
>> Add s337m probing/reading similarly to spdif.
>> ---
>> libavformat/wavdec.c | 23 +++
>> 1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
>> 575c667452..d030ed9f9d 100644
>> --- a/libavformat/wavdec.c
>> +++ b/libavformat/wavdec.c
>> @@ -41,6 +41,7 @@
>> #include "riff.h"
>> #include "w64.h"
>> #include "spdif.h"
>> +#include "s337m.h"
>>
>> typedef struct WAVDemuxContext {
>> const AVClass *class;
>> @@ -55,15 +56,17 @@ typedef struct WAVDemuxContext {
>> int audio_eof;
>> int ignore_length;
>> int spdif;
>> +int s337m;
>> int smv_cur_pt;
>> int smv_given_first;
>> int unaligned; // e.g. if an odd number of bytes ID3 tag was prepended
>> int rifx; // RIFX: integer byte order for parameters is big
>> endian } WAVDemuxContext;
>>
>> -static void set_spdif(AVFormatContext *s, WAVDemuxContext *wav)
>> +static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav)
>> {
>> -if (CONFIG_SPDIF_DEMUXER && s->streams[0]->codecpar->codec_tag == 1) {
>> +AVCodecParameters *par = s->streams[0]->codecpar;
>> +if ((CONFIG_SPDIF_DEMUXER || CONFIG_S337M_DEMUXER) &&
>> + par->codec_tag == 1) {
>
>Did you test this with both "--disable-everything --enable-demuxer=spdif" and
>"--disable-everything --enable-demuxer=s337m"?
>Neither should fail compilation.
>
>Carl Eugen
I just have tested both, and compilation is ok.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 7/8] avformat/wavdec: fix s337m/spdif probing beyond data_end
>>
>> ---
>> libavformat/wavdec.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
>> 3571733817..d8a27c79cf 100644
>> --- a/libavformat/wavdec.c
>> +++ b/libavformat/wavdec.c
>> @@ -77,7 +77,7 @@ static void set_spdif_s337m(AVFormatContext *s,
>> WAVDemuxContext *wav)
>> ret = AVERROR(ENOMEM);
>> } else {
>> int64_t pos = avio_tell(s->pb);
>> -len = ret = avio_read(s->pb, buf, len);
>> +len = ret = avio_read(s->pb, buf, FFMIN(len,
>> + wav->data_end - pos));
>
>Sorry if this was already answered:
>What exactly does this fix? Is it possible that avio_read() overreads without
>this check?
>
>Carl Eugen
There is no severe low level avio overread risk, but there is a risk to read
bytes that are not audio data in case audio data is followed by other data (a
RIFF chunk that would come just after the audio data RIFF chunk).
Here is a sample a build : http://0x0.st/zhiO.wav This sample is detected as
ac3 whereas the sync codes are beyond the data chunk, in a 'junk' (I called it
this way) chunk that is not supposed to be read (ex: the duration can be
cross-checked with sox or audacity for example).
But again, this is obviously very improbable "in the nature" as they are three
conditions :
- the wav file must be short enough (for the probe to seek in the end of the
file)
- the audio data chunk must be followed by another chunk with misc data
(usually those are considered "header metadata" and stand before it)
- the misc data chunk must emulate the sync codes
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/9] avformat/s337m: Use base AVClass for av_log usage
>> fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier:
>> > s337m_get_offset_and_codec does not make use of
>> > AVFormatContext: AVClass is enough for logging.
>> > Will facilitate further use from outside
>> > ---
>> > libavformat/s337m.c | 10 +-
>> > 1 file changed, 5 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/libavformat/s337m.c b/libavformat/s337m.c index
>> > 48ab66a6da..8956afb23f 100644
>> > --- a/libavformat/s337m.c
>> > +++ b/libavformat/s337m.c
>> > @@ -31,7 +31,7 @@
>> > #define IS_24LE_MARKER(state) ((state & 0x) == MARKER_24LE)
>> > #define IS_LE_MARKER(state) (IS_16LE_MARKER(state) ||
>> > IS_20LE_MARKER(state) || IS_24LE_MARKER(state))
>> >
>> > -static int s337m_get_offset_and_codec(AVFormatContext *s,
>> > +static int s337m_get_offset_and_codec(void *avc,
>> >uint64_t state,
>> >int data_type, int data_size,
>> >int *offset, enum AVCodecID
>> > *codec) @@ -50,8 +50,8 @@ static int
>> > s337m_get_offset_and_codec(AVFormatContext *s,
>> > }
>> >
>> > if ((data_type & 0x1F) != 0x1C) {
>> > -if (s)
>> > -avpriv_report_missing_feature(s, "Data type %#x in SMPTE
>> > 337M", data_type & 0x1F);
>> > +if (avc)
>> > +avpriv_report_missing_feature(avc, "Data type %#x in
>> > + SMPTE 337M", data_type & 0x1F);
>> > return AVERROR_PATCHWELCOME;
>> > }
>> >
>> > @@ -72,8 +72,8 @@ static int s337m_get_offset_and_codec(AVFormatContext *s,
>> > *offset = 1601;
>> > break;
>> > default:
>> > -if (s)
>> > -avpriv_report_missing_feature(s, "Dolby E data size %d in
>> > SMPTE 337M", data_size);
>> > +if (avc)
>> > +avpriv_report_missing_feature(avc, "Dolby E data size
>> > + %d in SMPTE 337M", data_size);
>> > return AVERROR_PATCHWELCOME;
>> > }
>> >
>>
>> Should be ok
>
>should i apply this or wait for the rest of the set ?
In my opinion, it can be applied. The idea is to have something consistent with
later introduced ff_s337m_get_packet() parameters and those have not been
questioned in the current review.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/9] avformat/wavdec: fix s337m/spdif probing beyond data_end
> > >> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
> > >> 2796905e1f..ccb9576b84 100644
> >> --- a/libavformat/wavdec.c
> > >> +++ b/libavformat/wavdec.c
> > >> @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s,
> > >> WAVDemuxContext *wav)
> > >> ret = AVERROR(ENOMEM);
> > >> } else {
> > >> int64_t pos = avio_tell(s->pb);
> > >> -len = ret = avio_read(s->pb, buf, len);
> > >> +len = ret = avio_read(s->pb, buf, FFMIN(len,
> > >> + wav->data_end - pos));
> > >> if (len >= 0) {
> > >> ret = ff_spdif_probe(buf, len, );
> > >> if (ret > AVPROBE_SCORE_EXTENSION) {
> > >
> > >Looks OK. I suppose this fixes a potential SIGSEGV too?
> > AVIO would just stop at EOF, I don't think a SIGSEGV could occur in any
> > scenario.
> > This fix only affects probing (ie. reading start of file) in a surprising
> > scenario where the data chunk would not extend to the end of the file.
> > This is many IF and I find it unlikely, but I think it should be fixed
> > anyway.
>
> Could you elaborate?
Here is a sample a build : http://0x0.st/zhiO.wav
This sample is detected as ac3 whereas the sync codes are beyond the data
chunk, in a 'junk' (I called it this way) chunk that is not supposed to be read
(ex: can be cross-checked with sox or audacity for example).
But again, this is obviously very improbable "in the nature" as they are three
conditions :
- the wav file must be short enough
- the data chunk must be followed by an additional header chunk (usually those
headers stand before it)
- the data chunk must emulate the sync codes
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 4/9] avformat/s337m: New ff_s337m_probe()
>Since you add an option in a later patch: Can you explain the >reasoning for the whole patchset better? >If DolbyE can be auto-detected (I assume so), this should be >done and no further option should be needed. > >Carl Eugen A common use case is stream copying : you want to be able to forward the input dolbyE to the output. But the mxf mux, for example, does not support dolby_e (up to now). In the future, if we come to support muxing dolby_e in s337m, then yes, that would be nice, we would be able to remux a dolby_e and fix its position/guard band, that would be a very interesting repairing workflow, but that would require much hard work (DolbyE recommanded position is dependant on video raster etc.). Second thing is that, even if we were able to support stream copying, the fact is that users have tons of script like this : ffmpeg -i input.mxf -vcodec copy output.mxf I mean broadcasters are often missing the "audio codec copy" parameter because they think "my audio is uncompressed 16 bits, stream copying is bitexact the same as transcoding". I am afraid I have myself many scripts like this... So, it is necessary to default to not-probe/decode dolby_e to not break scripts. And most of the time, the fact is that users just want to pass trough the data as they indeed need to be agnostic. This was the reason to choose to 1) add an AVOption, and 2) makes it default to disabled Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6 0/5] Fix mpeg1/2 stream copy
>Apart from what Anton wrote, it appears to me that not all patches in the >series are necessary to fix the stream-copying. > >Carl Eugen Precisely: - 1/2/3 are working together : actual code from ffmpeg.c that is used for stream encoding is now used for probing in libav, so it is "moved to public" and shared. But strictly speaking, 3 is not required to fix the stream-copying, it is only a "code simplification" = use the newly created public method. - 4 is a bug fix that is necessary to apply before 5 to avoid generating duplicate side data (this does not prevent stream copy from working, but of course this is very bad), but maybe I should have indicated this in the commit msg - 5 just add the coded side data that find_stream_info will forward to stream side data that ffmpeg will forward to output mux in case of stream copy. Sorry, I have little experience in contributing to open projects and I think I tend to not document enough in the commit msgs Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6 1/5] avformat: Add av_stream_add_coded_side_data()
>If the intention is to use this code from avformat_find_stream_info(), >then why should it be public? The documentation is very unclear as to >when/how an API user is supposed to call it. > >Anton Khirnov Sorry about that, I should have explained this in the cover letter, or maybe even in the commit msg. The reason is that this code already exists in ffmpeg.c (executed in case of non-codec copy if I remember correctly) and now I need it to be shared and used in avformat_find_stream_info also (for probing in general, and for codec copy in particular) to avoid code duplication. The fact is that initially, I had regrouped all this refactoring in a single commit, thus resulting in a clear full picture but it was not correct to mix libav and fftools changes, so I split the commit but an explanation was lacking, I missed that, sorry. I propose to describe this in the commit msg, this should be enough as the code/usage itself already exists in ffmpeg.c so maybe it is not necessary to add public documentation. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 9/9] avformat/wavdec: Test s337m
>> Test the new 'dolbyeprobe' AVOption.
>> Test dolby_e decoding for 24 bits with program config '5.1+2'
>> ---
>> tests/Makefile | 1 +
>> tests/fate-run.sh| 4
>> tests/fate/audio.mak | 5 +
>> 3 files changed, 10 insertions(+)
>>
>> diff --git a/tests/Makefile b/tests/Makefile index
>> e5f41008d4..65cccac312 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -75,6 +75,7 @@ ENCDEC2 = $(call ALLYES, $(firstword $(1))_ENCODER
>> $(lastword $(1))_DECODER \
>> $(firstword $(3))_MUXER $(lastword $(3))_DEMUXER)
>>
>> DEMDEC = $(call ALLYES, $(1)_DEMUXER $(2:%=%_DECODER))
>> +DEMDEMDEC = $(call ALLYES, $(1)_DEMUXER $(2)_DEMUXER
>> +$(3:%=%_DECODER))
>> ENCMUX = $(call ALLYES, $(1:%=%_ENCODER) $(2)_MUXER)
>>
>> DEMMUX = $(call ALLYES, $(1)_DEMUXER $(2)_MUXER) diff --git
>> a/tests/fate-run.sh b/tests/fate-run.sh index 83cad8cabe..f06b2fd029
>> 100755
>> --- a/tests/fate-run.sh
>> +++ b/tests/fate-run.sh
>> @@ -162,6 +162,10 @@ pcm(){
>> ffmpeg "$@" -vn -f s16le -
>> }
>>
>> +dolbye2pcm16(){
>> +ffmpeg -dolbyeprobe 1 "$@" -vn -f s16le - }
>> +
>> fmtstdout(){
>> fmt=$1
>> shift 1
>> diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak index
>> c41958ea2d..0e56d401ea 100644
>> --- a/tests/fate/audio.mak
>> +++ b/tests/fate/audio.mak
>> @@ -24,6 +24,11 @@ fate-dolby-e: CMD = pcm -i
>> $(TARGET_SAMPLES)/dolby_e/16-11
>> fate-dolby-e: CMP = oneoff
>> fate-dolby-e: REF = $(SAMPLES)/dolby_e/16-11.pcm
>>
>> +FATE_SAMPLES_AUDIO-$(call DEMDEMDEC, WAV, S337M, DOLBY_E) +=
>> +fate-dolby-e-wav
>> +fate-dolby-e-wav: CMD = dolbye2pcm16 -i
>> +$(TARGET_SAMPLES)/dolby_e/512.wav
>> +fate-dolby-e-wav: CMP = oneoff
>> +fate-dolby-e-wav: REF = $(SAMPLES)/dolby_e/512.wav.pcm
>> +
>> FATE_SAMPLES_AUDIO-$(call DEMDEC, DSS, DSS_SP) += fate-dss-lp
>> fate-dss-sp
>> fate-dss-lp: CMD = framecrc -i $(TARGET_SAMPLES)/dss/lp.dss -frames
>> 30
>> fate-dss-sp: CMD = framecrc -i $(TARGET_SAMPLES)/dss/sp.dss -frames
>> 30
>>
>This is missing some kind of hash check on the demuxed data
The "oneoff" tests consists in checking the maximum difference between the raw
pcm output samples, it must be 0 or 1 max. This test is done in 16-bit
truncated output of the decoded stream.
It raises an error too if the duration does not strictly match. I found it
appropriate (a strict hash on decoded samples may also break with the many
floats of the DolbyE decoder).
My idea was to keep a single test for both "wav demux" and "5.1+2" decode.
Do you think there should be an additional test focused on demuxed data ?
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 7/9] avformat/wavdec: Fix s337m last packet parsing
>> if (CONFIG_SPDIF_DEMUXER && wav->spdif == 1)
>> return ff_spdif_read_packet(s, pkt);
>> -if (CONFIG_S337M_DEMUXER && wav->s337m == 1)
>> -return ff_s337m_read_packet(s, pkt);
>>
>> if (wav->smv_data_ofs > 0) {
>> int64_t audio_dts, video_dts; @@ -712,6 +710,10 @@ smv_out:
>> wav->data_end = avio_tell(s->pb) + left;
>> }
>>
>> +if (CONFIG_S337M_DEMUXER && wav->s337m == 1) {
>> +size = FFMIN(S337M_MAX_OFFSET, left);
>> +ret = ff_s337m_get_packet(s->pb, pkt, size, NULL, s,
>> st->codecpar->bits_per_coded_sample);
>> +} else {
>
>Couldn't you roll this into the patch that adds the call to
>ff_s337m_read_packet()?
OK. I was not sure about it. I thought it might have been interesting to keep
s337m as close as possible to spdif as long as possible, and then only fork at
the very end with this last patch, but maybe it is does not make so much sense.
Thus, I will move ff_s337m_read_packet() back to a static s337m_read_packet(),
I think this is better as indeed s337m_read_packet should never be used from
outside when s337m is submuxed in another mux because it does not restrict the
available size that could be read from avio.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/9] avformat/wavdec: fix s337m/spdif probing beyond data_end
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
>> 2796905e1f..ccb9576b84 100644
>> --- a/libavformat/wavdec.c
>> +++ b/libavformat/wavdec.c
>> @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s,
>> WAVDemuxContext *wav)
>> ret = AVERROR(ENOMEM);
>> } else {
>> int64_t pos = avio_tell(s->pb);
>> -len = ret = avio_read(s->pb, buf, len);
>> +len = ret = avio_read(s->pb, buf, FFMIN(len,
>> + wav->data_end - pos));
>> if (len >= 0) {
>> ret = ff_spdif_probe(buf, len, );
>> if (ret > AVPROBE_SCORE_EXTENSION) {
>
>Looks OK. I suppose this fixes a potential SIGSEGV too?
AVIO would just stop at EOF, I don't think a SIGSEGV could occur in any
scenario.
This fix only affects probing (ie. reading start of file) in a surprising
scenario where the data chunk would not extend to the end of the file.
This is many IF and I find it unlikely, but I think it should be fixed anyway.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/9] avformat/s337m: Consider container bit resolution
>> +if (container_word_bits && (container_word_bits+7)/8 !=
>> + (word_bits+7)/8) {
>
>Can it happen that word_bits is anything but 16 or 24 with a valid stream? If
>not then I'd check container_word_bits == word_bits && (word_bits == 16 ||
>word_bits == 24) or so
word_bits may be 20, and in that case container_word_bits must be 24 (this is
the case in my fate test), so I think this is correct.
>> +while ((container_word_bits == 24 || !IS_16LE_MARKER(state))
>> +&& (container_word_bits == 16 || !IS_20LE_MARKER(state) &&
>> + !IS_24LE_MARKER(state))) {
>
>I'd rewrite this as while ((bits == 24 && (20LE || 24LE)) || (bits ==20 &&
>16LE)), more readable
container_word_bits may be 0 for autodetect, this results in this expression...
I agree it is not that great for readability, but doing otherwise would require
some additions and macros duplications, for example:
while ( !(!bits && LE) || !(bits == 24 && (20LE || 24LE)) || !(bits ==16 &&
16LE))
Sounds heavy, not sure this is really better ?
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 3/7] avformat: Add av_stream_add_coded_side_data()
>> #define LIBAVFORMAT_VERSION_MAJOR 58 >> -#define LIBAVFORMAT_VERSION_MINOR 35 >> +#define LIBAVFORMAT_VERSION_MINOR 36 >> #define LIBAVFORMAT_VERSION_MICRO 101 >> >You forgot to reset micro. > >- Andreas Sorry, I just checked the git log and now understand how micro versions works. I will send a v3 with micro reset to 100. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/5] fftools/ffmpeg: Fix forward CPB props in to out
>> @@ -3562,12 +3562,14 @@ static int init_output_stream(OutputStream *ost,
>> char *error, int error_len)
>> int i;
>> for (i = 0; i < ist->st->nb_side_data; i++) {
>> AVPacketSideData *sd = >st->side_data[i];
>> +if (sd->type != AV_PKT_DATA_CPB_PROPERTIES) {
>> uint8_t *dst = av_stream_new_side_data(ost->st, sd->type,
>> sd->size);
>> if (!dst)
>> return AVERROR(ENOMEM);
>> memcpy(dst, sd->data, sd->size);
>> if (ist->autorotate && sd->type ==
>> AV_PKT_DATA_DISPLAYMATRIX)
>> av_display_rotation_set((uint32_t *)dst, 0);
>> +}
>> }
>> }
>>
>
>Can you clarify why you are exlcuding CBP side-data from being copied here? It
>seems to not match the commit message in my mind.
>Note that from some containers, namely mov/mp4, the container can actually
>provide CBP data, which means the stream CBP data would be valid and
>potentially relevant.
This portion of the code is for when the stream is transcoded, so there is no
relation between the input and output CPB.
But in init_output_stream_streamcopy() you can see that all side data is
actually copied (it was already there, before this patchset, and I left it
untouched) with no exception (=incl. CPB). And this is what I was looking for:
being able to forward the input CPB size to the output, when stream-copying.
Note I only modified the CPB behaviour to restrict the scope of this patchset,
but there are certainly other side data that should not be forwarded from input
to output; this does not sound easy for me and maybe it should be discussed,
but this patchset is consistent as is and I think it would require other
patches if we want something more generic about what side data should be
forwarded or not.
This is my understanding and I may have missed something of course. I have
mainly tested mpeg2 encoding and stream-copying and checked fate.
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avformat/mpegenc.c: vbvsize option
>>On Thu, Oct 31, 2019 at 06:04:58PM +0100, Nicolas Gaullier wrote: >> Allow the user to set or override the vbv size >> --- >> libavformat/mpegenc.c | 6 +- >> 1 file changed, 5 insertions(+), 1 deletion(-) > >This is not the "correct" way to handle this, because one mpeg container >can contain many streams and this is just one parameter for the container. >while the vbvsize is a parameter per stream. FYI, my main concern/use case is to be able to rewrap an input mpg file to a new mpg file while keeping its vbv size. This was working back in version 2.8.9, just before codecpar was introduced (ie. regression)... My first proposal would be very straightforward : insert a new "rc_buffer_size" field at the end of AVCodecParameters but that would break the ABI? Another point is that rc_buffer_size may be carried by stream side_data when encoding, so that would mean two representations for the same information, I don't know if this sounds "correct". At the end, do you think amending AVCodecParameters would be acceptable ? The fact is, I have no other idea because I don't see anyway how to setup stream side_data from within the codec of the input stream. Thank you for your review and ... help Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avcodec/h264: Fix poc_lsb in open gop context
>De : ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] De la part de >Kieran Kunhya >Envoyé : lundi 2 septembre 2019 18:12 >Objet : Re: [FFmpeg-devel] [PATCH 1/1] avcodec/h264: Fix poc_lsb in open gop >context > >Patch seems good. > >Kieran Can someone please apply the patch ? Thanks Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] avcodec/h264: Fix poc_lsb in open gop context
>Envoyé : mardi 23 juillet 2019 14:09 >When no IDR nor mmco_reset is found, prev_poc_lsb is undefined and shall not >be assumed to be zero >--- > libavcodec/h264_parse.c | 2 ++ > libavcodec/h264dec.c| 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) I have not received any feeback, do you think this patch could be applied ? Thank you, Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 3/6] avformat/s337m: Make available as subdemuxer
>Look at spdif in wav. Indeed, I missed that, spdif and s337m are very similar technically, and it means I have to rework my patch for the integration of s337m in wavdec, but I still do not catch why another demuxer should be created : spdifdec.c is actually quite similar to the existing s337m.c Perhaps you find unacceptable only the use of the avpriv_ prefix instead of ff_ ? I understand, but in a previous cover letter, I mentioned that the idea was to make this available to build a filter that would decode s337m in a filter graph; but yes, maybe I should stay with ff_ for now and migrate to avpriv_ later when submitting the new filter, that would make sense, moreover the prototype may be changed too depending on how the implementation of the filter turns to be (and I have no experience in writing a filter for ffmpeg for now, so...). In my opinion, here are the most relevant differences between spdif and s337m, it is a matter of use cases : - s337m may be split in two mono streams and may require an "audio merge" to be decodable, which suggests the use of a filter graph and a dedicated filter to decode s337/dolby_e (so the use of public methods for a dedicated format, indeed) - dolby E is a codec one either don't want to see (ie. pass through) or one want to get rid off/decode to uncompressed/pcm, whereas ac3 for example is a very popular codec one usually wants to stream copy: so an audio filter to decode s337/dolbyE seems interesting whereas an audio filter to decode spdif/ac3 would be absolutely useless - s337m may be frame wrapped (ex: mxf) (probing method differs) At the end, in my current understanding, here is what I should correct : - make the integration of s337m in wavdec similar to that of spdif - replace the avpriv_* by ff_* ... for now Anyway, I will wait your feedback and until we completely agree before going anywhere further. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 4/6] avformat/wavdec: s337m support
>.. this won't compile if S377m is disabled. Just go with the old #if
>solution, sorry for causing confusion
My mistake but I think it is a good idea to get rid of some bulky #if... Do you
think a mixed usage of if and #if would be acceptable ? I think the following
would be the best :
if (!CONFIG_S337M_DEMUXER || st->codecpar->codec_id != AV_CODEC_ID_DOLBY_E)
{
(...)
#if CONFIG_S337M_DEMUXER
} else {
(... using s337m_get_packet method ... )
#endif
}
NB: I have to rework all the patchset with carl's feedback, but this point will
still remain, so better to have it solved now.
Thank you
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 3/6] avformat/s337m: Make available as subdemuxer
>> +int avpriv_s337m_probe_stream(void *avc, AVIOContext *pb, AVStream **st, >> int size); >Sorry for not commenting earlier: You're welcome, a few days is not that much, particularly in the middle of the summer! >This is to the best of my knowledge not acceptable (to have a public function >for a particular format), instead create a new demuxer that supports this raw >format. S337m is a very particular format though: potentially, it may be probed within any pcm stream, I don't see anything comparable in any other format. The problem is, the developer will use a standard demuxer or device to open the streams (for example, a decklink, an mxf etc.), and then decide if he wants to process some of the streams as regular pcm or as dolby E to be decoded. I may be wrong, but I don't see how the same workflow could be obtained by creating a new separate demuxer. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 3/6] avformat/s337m: Make available as subdemuxer
>Ah, so it's time and not phase? Renaming the #defines to make that clearer >would be nice. Other than that, this isn't a huge issue I assume the start of the packet to be synched with the start of "a" video frame, I think there is no use case where pcm and video are not synced, at least when dolby E is concerned. So, DOLBY_E_PHASE_MIN/MAX are phase and well named I think. Concerning S337M_PHASE_PROBE_MIN that I renamed S337M_PROBE_GUARDBAND_MIN_BYTES, this is still a phase, a kind of safe guard phase, to make even more unlikely to have a wrong detection. But the question is to determine if this phase should be specified in milliseconds or just in raw bytes (or another man could say in number of samples whether there are 16 or 24 bits). There may be other approaches, but if we consider strictly the risk of statistical match, it is related to the number of raw bytes, this is why I choosed raw bytes at the end and thus the name S337M_PROBE_GUARDBAND_MIN_BYTES looks good to me. Furthermore, at the end, this S337M_PROBE_GUARDBAND_MIN_BYTES is zero for now (so whatever the unit), and in my experience, this is generally required as there are some programs which indeed have a zero-phase Dolby E (and BTW it appears some softwares doesn't like it). Taking the worst case scenario (16 bits), the sync code takes 4 bytes, so the raw probability is only 1/2^32 (and even less taking into account that usually broadcast programs starts with a little bit of silence). So anyway, maybe I took too much care on this, but at least here this is explicit in the code, I think. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 3/6] avformat/s337m: Make available as subdemuxer
>> +if (pos < size - 9 && pos >= S337M_PROBE_GUARDBAND_MIN_BYTES)
>I think this 9 should be an 11 or 12..
Indeed, thank you, my mistake.
>This isn't quite what I meant by turning it into an integer test :) this will
>likely just be rounded to zero. I meant that things could likely be rearranged
>so there's no divisions, so the probing isn't subject to the vaguaries of
>float >rounding
On one side, dealing with a macro that contains a double
(S337M_PHASE_PROBE_MIN) makes it impossible at the end to avoid a test
involving float rounding (in my understanding, but I may be wrong, and I
remember there are some helpers in ffmpeg to make it easier to avoid float
rounding), or maybe I should have limited the digits and simply worked with an
integer number of milliseconds, but that sounded a little bit overkill,
together with the fact that the default value is still 0. so all of this is
only in case a developer wants to change the #define.
On the other side, I don't think it was such a good idea to specify this in
seconds as for the dolby_e min/max. This value is just for assurance that there
will be no wrong probing, and I thought that just byte count was more
appropriate... and that makes it an integer test at the end :)
So I replaced :
if (pos < size - 9 &&
(s337m_phase = (double)pos * 4 / (*st)->codecpar->bits_per_coded_sample /
(*st)->codecpar->sample_rate) >= S337M_PHASE_PROBE_MIN) {
With just simply :
if (pos >= S337M_PROBE_GUARDBAND_MIN_BYTES)
And thus replaced S337M_PHASE_PROBE_MIN=0.000 by
S337M_PROBE_GUARDBAND_MIN_BYTES=0.
At the end, I don't see any cons to do that, and it is far most simplest.
> +#define DOLBY_E_PHASE_MIN 0.000610
> +#define DOLBY_E_PHASE_MAX 0.001050
>Where do these phase values come from? Is there a spec somewhere?
https://www.dolby.com/us/en/technologies/dolby-e-line-position.pdf
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 5/5] avformat/s337m: Test wav subdemux
>> +FATE_SAMPLES_AUDIO-$(call DEMDEMDEC, WAV, S337M, DOLBY_E) += >> +fate-dolby-e-wav >> +fate-dolby-e-wav: CMD = dolbye2pcm16 -i >> +$(TARGET_SAMPLES)/dolby_e/512.wav >> +fate-dolby-e-wav: CMP = oneoff >> +fate-dolby-e-wav: REF = $(SAMPLES)/dolby_e/512.wav.pcm > >This does a floaty comparison, right? Since the Dolby-E decoder is float at >the moment.. The test is similar to the current "fate-dolby-e", this new one is only different because the input dolby E stream is 24 bits (instead of 16), contains 5.1+2 (instead of 5.1)... and uses wav wrapper of course. In both case, the comparison is based on the truncated 16-bit output of the Dolby-E decoder (but I think the inner precision of the codec is higher) AND a tolerance of +/-1. At the end, I think there is no rounding/platform-dependant issue, if this is your concern. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avformat: Support s337m in mxf/wav/w64
> > > MXF has a UL registered for Dolby-E: > I also notice this UL is already in mxf.c, but added commented out by > Baptiste back in 2006.. There are two ways for signalling dolby-E in MXF : channel status IF using an AES3 descriptor, or the registered UL for Sound Essence Coding. ARD/ZDF/HDF requires both but it is very specific. The use of the channel status only seems a little bit more common, I have seen it in Harmonic files. But at the end, what is really highly widespread is to have no signaling at all. > So, a general solution is needed. When the user knows the audio is Dolby-E > then they can obviously just override the decoder, if they want (not always). > Another typical case is "if you think the audio is Dolby- E, automatically > decode it so I don't blow my speakers!". An option could allow > avformat_find_stream_info() to wait for the first audio packet and probe it, > and not take what the demuxer says at face value. Concerning a general solution, the problem is even worse : nowadays MXF files are mostly structured with mono audio tracks (ARD/ZDF, AS-10 in France etc.)... My idea was to enable the submux only for wav/mxf/potentially quicktime because I don't see use case for other formats; but this require stereo audio. For a more general solution (MXF with mono audio tracks specifically), I have in mind to build an audio filter : a graph would merge the L/R to build a dolbyE stream that the filter could decode: this is not very handy for users, but I don't know how it could be done otherwise. I am currently working with an AVOption in MXF contexts, but if you think avformat_find_stream_info() is a better way, I will take a look... but I am not very confident with my knowledge of ffmpeg for that work, and I will rather certainly concentrate on the general case with the audio filter if this is valid for you ? Thank you for all your comments Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] lavfi/silencedetect v3
Hello,
I have not received any comment yet on my patchset v3 ("add mono mode" new
feature + 4 fixes including a ticket), does it look good to you, could it be
committed ?
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225494.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225495.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225496.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225497.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225498.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225499.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225500.html
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225501.html
Thanks,
Nicolas Gaullier
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] lavfi/silence_detect: V3 (updated commit messages)
I have updated all the commit messages, and the typo in patch 6/8 is now clean up - there was no other comment so far. Please review Nicolas Gaullier ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 6/8] avfilter/silencedetect: fix for ticket 6968 Fix missing log of silence_end at end of stream
> +av_ts2timestr(duration_ts, _base)); ^ this does not build, and looks like a typo Sorry, this is ugly, I had indeed replaced manually two occurrences (s->time_base => _base) at the last minute just before sending the email... By the way, at the end of this set of patch, I am wondering if a "somewhat cosmetic" patch which would now remove some 'time_base' local arguments by using the s->time_base from the context would be interesting to have the code little more consistent. I send right now the fixed patch as v3 including carl eugen's feedback on commit msg. Thank you Nicolas Gaullier ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 6/8] avfilter/silencedetect: fix for ticket 6968 Fix missing log of silence_end at end of stream
Here, the "fix for ticket 6968" should be the header, so I think the format is
correct ? Everything behind ("Fix missing...") should be the body of the
commit message.
NB: thanks to you to have reported it previously, I was not aware of this
ticket when I started work on this issue!
Nicolas Gaullier
-Message d'origine-
De : ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] De la part de Carl
Eugen Hoyos
Envoyé : lundi 12 février 2018 14:52
À : FFmpeg development discussions and patches
Objet : Re: [FFmpeg-devel] [PATCH v2 6/8] avfilter/silencedetect: fix for
ticket 6968 Fix missing log of silence_end at end of stream
2018-02-12 10:48 GMT+01:00 Nicolas Gaullier :
> From: nicolas gaullier
Please mention ticket #6968 in the commit message if it is related.
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 1/8] avfilter/silencedetect: add mono mode in mono mode, silence is detected in any single channel (instead of all them simultaneously)
>I believe there is an issue with your commit message: >It should be something like: >lavfi/silencedetect: Add mono mode > >In mono mode, silence is detected... >(Note the two linebreaks, other differences are irrelevant.) > >Carl Eugen Sorry, all my patches are missing an empty newline to separate the header and the body of the commit messages, I just realize my mistake, thank you. I will fix that in the next post at the end of the current review. Nicolas Gaullier ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] avformat/utils : estimate_timings_from_pts - increase retrys
>> The number of extra retries to get more than a single duration is currently >> limited to 1 (see commit 77a0df4b5). >> This patch raises this number of retries to 4 (amongst the overall 6 max >> retries). >> In my experience, the need for 2 extra retries is something commonplace. >> 3 extra retries is not far away and we have an exemple on ffmpeg.org : >> http://samples.ffmpeg.org/archive/extension/ts/mpegts+mpeg2video+ac3++ >> mpegts_multiple_ts_packets_pmt_pid_0x50.ts >This would make the code read much more of the file if it cannotz figure out >the duration of every stream. >In which use case are the individual stream durations needed ? > >btw, while 4 retries might sound low, each reads twice as much data than the >previous IIRC The use case is probing, and especially quickly determining the exact number of video frames (guessing ptses are correct), but indeed, it is useless for a regular ffmpeg transcoding command. Maybe a better option would be to change the "#define DURATION_MAX_READ_SIZE" by an overridable command line option ? Then, when using for example ffprobe, I could raise the max readsize to 4Mo to cover my worst case scenario without affecting the default behaviour. Thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] avformat/utils : estimate_timings_from_pts - increase retrys
The number of extra retries to get more than a single duration is currently limited to 1 (see commit 77a0df4b5). This patch raises this number of retries to 4 (amongst the overall 6 max retries). In my experience, the need for 2 extra retries is something commonplace. 3 extra retries is not far away and we have an exemple on ffmpeg.org : http://samples.ffmpeg.org/archive/extension/ts/mpegts+mpeg2video+ac3++mpegts_multiple_ts_packets_pmt_pid_0x50.ts I have encountered at least one time a file (SPTS) that requires 4 extra retries, this is why I propose to set this value. Thank you for your review Nicolas Gaullier 0001-avformat-utils-estimate_timings_from_pts-increase-re.patch Description: 0001-avformat-utils-estimate_timings_from_pts-increase-re.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start
>this breaks fate > >If the changes are intended the reference must be updated by the patch causing >the changes Sorry, forgot... Now, I have some questions regarding fate tests: 1) I would like to update the fate test itself : Currently, we have : silencedetect=d=-20dB I am considering changing it to : silencedetect=n=-30dB:d=.4 The reason is that the usage would be more relevant (dB applying to noise + duration set to a consistent value for this speech sample), easier to check manually in a waveform editor, and that the coverage would be extended for the new patches (silence_start=0 + log of silence_end at end of stream). Should I first publish the patch with only the fate results changed and later on another patch to update the fate test with results changed again ? Personally, I would say a single patch with all three items (patch + fate test update + fate result update) would be clearer, but I am not familiar with ffmpeg usages, so I prefer asking... 2) I just realized today that I could fix the accuracy of silence_end too, even if it is clearly much less important compared to silence_start Do you think this should be handled by another patch, or should I better group this patch with this one as they both deal with time accuracy and affect fate results (I would rather go for the latter) ? 3) Should I prepare a new fate test to cover the new "mono" mode (patch 1/4) ? Thank you! Nicolas Gaullier ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start
(attachement + email object fixed : sorry about that x2) Attached patch fixes accuracy of silence_start. The benefit is mostly noticeable when the silence starts at the very beginning (ie. silence_start=0 exactly). Nicolas Gaullier 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch Description: 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start)
(email object fixed : sorry about that) Attached patch fixes accuracy of silence_start. The benefit is mostly noticeable when the silence starts at the very beginning (ie. silence_start=0 exactly). Nicolas Gaullier ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/4] af_silencedetect : fix missing log silence_end at EOS
Attached patch fixes missing log of "silence_end" when the silence continue until the end of the stream. Nicolas Gaullier 0004-avfilter-af_silencedetect-fix-missing-log-silence_en.patch Description: 0004-avfilter-af_silencedetect-fix-missing-log-silence_en.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/4] af_silencedetect : add mono mode
Hi! Attached patch adds a "mono mode", I mean detection of silence in any of the channels (instead of all of them simultaneously). The channel number is detected and logged, and side metadata names are suffixed with the channel number (NB: in non-mono mode, the logging and metadata remain unchanged). This first patch includes minor technical changes to prepare the following patches, I apologize but I thought it was the best way to present this set of patches. Thank you for the review Nicolas Gaullier 0001-avfilter-af_silencedetect-add-mono-mode.patch Description: 0001-avfilter-af_silencedetect-add-mono-mode.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] af_silencedetect : fix when silence_start=0
Attached patch fixes silence_start=0 meaning "no silence start" Nicolas Gaullier 0003-avfilter-af_silencedetect-fix-silence_start-0-meanin.patch Description: 0003-avfilter-af_silencedetect-fix-silence_start-0-meanin.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] af_silencedetect : add mono mode
Attached patch fixes accuracy of silence_start. The benefit is mostly noticeable when the silence starts at the very beginning (ie. silence_start=0 exactly). Nicolas Gaullier 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch Description: 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter: add loudnorm
>Most broadcasters are now specifying mandatory loudness targets for source >material, which is usually supplied as a target integrated loudness value and >a target maximum true peak. Loudness normalization is easy for files with >sufficient headroom for linear gain adjustments, difficult for files with not >enough headroom, and even more difficult for live streams – this filter >handles all these situations. The loudnorm algo is basically a loudness-tuned >AGC which was designed to honor local dynamics, followed by a look-ahead true >peak limiter. Using normal parameters the result is usually imperceptible, >even with very dynamic source material such as classical music. This filter >should be a major boon to broadcasters and digital distribution people, since >the only software currently available that does this kind of thing is >commercial, and for the most part very expensive (Minnetonka, Skylark, Nugen, >etc.) +1 This is a very important and expected feature for broadcasters, I support that too! Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Support AS-10 MXF
>On Tue, 2016-03-29 at 22:29 +0200, NabFab wrote: >> Hi, >> Could you please tell me if ffmpeg is supporting AS-10 or AS-11 MXF >> format according to AMWA specifications ? >> Thank you > >Do you mean muxing or demuxing? I expect mxfdec should be perfectly capable of >demuxing. > >It's been a while since I looked at AS-10 but I recall it simply being a >narrower version of OP1a (but not as narrow as OPAtom). So just mux >something >with the codecs AS-10 expects and run it through an AS-10 analyzer to see if >mxfenc spits out the right thing. > >/Tomas To be compliant with AS10, it is required to insert a specific metadata (the ShimName, at least), this is quite some dev work, manipulating DMS Schemes... Body partitions are also restricted to 10s max, and there are many other constraints. For now, bmx is much more broadcast-oriented, already support as11 and has an as10 branch. The AMWA thing is all about certification and respect of the constraints to facilitate interoperability, it is not that easy to achieve. This said, AS10/11 are based on Op1A, and in some cases, people may say they "require" AS10/AS11 to "feel safe" despite the fact that more generic Op1A that ffmpeg can provide would probably be sufficient for their workflow to work. Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] MXF : default fied dominance is TFF
The field dominance is an optional property in the MXF specifications. It shall
be assumed to be 1 (top field first) if not present.
Nicolas
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 7a4633f..7517285 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -152,6 +152,7 @@ typedef struct {
#define MXF_TFF 1
#define MXF_BFF 2
int field_dominance;
+int field_dominance_present;
int channels;
int bits_per_sample;
unsigned int component_depth;
@@ -872,6 +873,7 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
break;
case 0x3212:
descriptor-field_dominance = avio_r8(pb);
+descriptor-field_dominance_present = 1;
break;
case 0x3301:
descriptor-component_depth = avio_rb32(pb);
@@ -1595,6 +1597,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
case MixedFields:
break;
case SeparateFields:
+if (!descriptor-field_dominance_present)
+descriptor-field_dominance = MXF_TFF;
switch (descriptor-field_dominance) {
case MXF_TFF:
st-codec-field_order = AV_FIELD_TT;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
I did not found an easy way to set up initialization values to properly handle
defaults but I am not a highly skilled developer, and maybe someone will find
how to implement this more elegantly.
They are also many other properties in mxf that are only optional, for example
component depth and horizontal/vertical subsampling factors that are actually
parsed, but as far from now it does not seem sufficiently useful to distinguish
between the initialization value '0' and 'not present'.
In my opinion, in the solely case of the field dominance, when it is found/set
to '0', it seems interesting to fail/raise a warning at least, but it is
somewhat particular and should not involve a big code refactoring to handle
this.
Nicolas
-Message d'origine-
De : ffmpeg-devel-boun...@ffmpeg.org [mailto:ffmpeg-devel-boun...@ffmpeg.org]
De la part de Carl Eugen Hoyos
Envoyé : lundi 8 septembre 2014 12:15
À : ffmpeg-devel@ffmpeg.org
Objet : Re: [FFmpeg-devel] MXF : default fied dominance is TFF
Gaullier Nicolas nicolas.gaullier at arkena.com writes:
case 0x3212:
descriptor-field_dominance = avio_r8(pb);
+descriptor-field_dominance_present = 1;
Is it possible to instead initialize
descriptor-field_dominance to MXF_TFF?
case SeparateFields:
+if (!descriptor-field_dominance_present)
+descriptor-field_dominance = MXF_TFF;
switch (descriptor-field_dominance) {
case MXF_TFF:
st-codec-field_order = AV_FIELD_TT;
Doesn't this allow to remove the default case below?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
