Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support
tis 2023-02-21 klockan 12:30 + skrev Nicolas Gaullier: > > This is why it is better to demand that these things be explicitly > > signalled. At the same time a lot of users expect ffmpeg to Just > > Work, but that is not always possible. Perhaps we should should put > > this in the manual and tell >spdif/s377m/dolby-e users to RTFM? > > > > > And another point here in this latest edition on my patch serie > > > is > > > that the use of an existing AVOption makes it possible for users > > > to > > > upgrade their command lines just now by adding the option : > > > ignored in previous version, it will take effect the day they > > > upgrade > > > their ffmpeg version, so the transition can be smooth... > > > > Assuming users read the documentation of course.. > > Yes, definitely, at the end, I don't see an option other than RTFM. > Documentation : I could insert a "dolby_e" section between ac3 and > flac in decoders.texi. > But there is nothing specific to dolby_e for real. > Maybe an overall "stream probing" chapter would be better in > decoders.texi between "Decoders" and "Video Decoders" chapters; with > a specific paragraph about the non_pcm_mode option for wav/s337 (and > later for s302) ? Hmm. Perhaps. This also touches demuxers to an extent, so perhaps a section for wavdec with a link to the dolby-e section in the decoder documentation in that case? /Tomas ___ 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/6] avformat/wavdec: s337m support
>This is why it is better to demand that these things be explicitly signalled. >At the same time a lot of users expect ffmpeg to Just Work, but that is not >always possible. Perhaps we should should put this in the manual and tell >>spdif/s377m/dolby-e users to RTFM? > >> And another point here in this latest edition on my patch serie is >> that the use of an existing AVOption makes it possible for users to >> upgrade their command lines just now by adding the option : >> ignored in previous version, it will take effect the day they upgrade >> their ffmpeg version, so the transition can be smooth... > >Assuming users read the documentation of course.. Yes, definitely, at the end, I don't see an option other than RTFM. Documentation : I could insert a "dolby_e" section between ac3 and flac in decoders.texi. But there is nothing specific to dolby_e for real. Maybe an overall "stream probing" chapter would be better in decoders.texi between "Decoders" and "Video Decoders" chapters; with a specific paragraph about the non_pcm_mode option for wav/s337 (and later for s302) ? 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/6] avformat/wavdec: s337m support
fre 2023-02-17 klockan 10:30 + skrev Nicolas Gaullier:
> > > +#if CONFIG_S337M_DEMUXER
> > > + {"non_pcm_mode", "Chooses what to do with s337m",
> > > OFFSET(non_pcm_mode), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC,
> > > "non_pcm_mode"},
> > > + {"copy" , "Pass s337m through unchanged", 0,
> > > AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 1, DEC, "non_pcm_mode"},
> > > + {"demux" , "Demux s337m" , 0,
> > > AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"},
> > > +#endif
> > > { NULL },
> > > };
> >
> > So default is to demux? Sounds OK and probably avoids eardrum
> > destruction
>
> Well, to be honest, I am not very comfortable with that.
> The fact is, I fear that many users have scripts to mux wav/dolby_e
> into mxf outputs and they will be affected...
This is why it is better to demand that these things be explicitly
signalled. At the same time a lot of users expect ffmpeg to Just Work,
but that is not always possible. Perhaps we should should put this in
the manual and tell spdif/s377m/dolby-e users to RTFM?
> And another point here in this latest edition on my patch serie is
> that the use of an existing AVOption makes it possible for users
> to upgrade their command lines just now by adding the option :
> ignored in previous version, it will take effect the day they upgrade
> their ffmpeg version,
> so the transition can be smooth...
Assuming users read the documentation of course..
>
> > > + } else {
> > > + av_log(s, AV_LOG_INFO, "Passing
> > > through S337M data: codec will not be reported\n");
> > > + }
> >
> > Perhaps the user should also be informed when S337m is demuxed
> > rather than passed through?
>
> I could add a debug message if you think that could be helpful ?
> I think I cannot add an INFO log as spdif and other probe-mecanisms
> are not verbose in current code.
Nah, it was more a rhetorical question
/Tomas
___
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/6] avformat/wavdec: s337m support
>> +#if CONFIG_S337M_DEMUXER
>> + {"non_pcm_mode", "Chooses what to do with s337m",
>> OFFSET(non_pcm_mode), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC,
>> "non_pcm_mode"},
>> + {"copy" , "Pass s337m through unchanged", 0,
>> AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 1, DEC, "non_pcm_mode"},
>> + {"demux" , "Demux s337m" , 0,
>> AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"},
>> +#endif
>> { NULL },
>> };
>
>So default is to demux? Sounds OK and probably avoids eardrum destruction
Well, to be honest, I am not very comfortable with that.
The fact is, I fear that many users have scripts to mux wav/dolby_e into mxf
outputs and they will be affected...
But I completely understand the ffmpeg logic to "always decode", as is done
currently in wav files to probe dts or even spdif which is really the same
thing as s337.
It does not seem reasonable to break this here.
And another point here in this latest edition on my patch serie is that the use
of an existing AVOption makes it possible for users
to upgrade their command lines just now by adding the option : ignored in
previous version, it will take effect the day they upgrade their ffmpeg version,
so the transition can be smooth...
>> + } else {
>> + av_log(s, AV_LOG_INFO, "Passing
>> through S337M data: codec will not be reported\n");
>> + }
>
>Perhaps the user should also be informed when S337m is demuxed rather than
>passed through?
>
>/Tomas
I could add a debug message if you think that could be helpful ?
I think I cannot add an INFO log as spdif and other probe-mecanisms are not
verbose 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 4/6] avformat/wavdec: s337m support
mån 2023-02-13 klockan 19:09 +0100 skrev Nicolas Gaullier:
> +#if CONFIG_S337M_DEMUXER
> + {"non_pcm_mode", "Chooses what to do with s337m",
> OFFSET(non_pcm_mode), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC,
> "non_pcm_mode"},
> + {"copy" , "Pass s337m through unchanged", 0,
> AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 1, DEC, "non_pcm_mode"},
> + {"demux" , "Demux s337m" , 0,
> AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"},
> +#endif
> { NULL },
> };
So default is to demux? Sounds OK and probably avoids eardrum
destruction
> + } else {
> + av_log(s, AV_LOG_INFO, "Passing
> through S337M data: codec will not be reported\n");
> + }
Perhaps the user should also be informed when S337m is demuxed rather
than passed through?
/Tomas
___
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 4/6] avformat/wavdec: s337m support
Add s337m probing and demuxing similarly to spdif.
Add 'non_pcm_mode' option to disable s337m demuxing (pass-through).
---
libavformat/wavdec.c | 47 +++-
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index e3f790fcc9..fd9ca89880 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -45,6 +45,7 @@
#include "riff.h"
#include "w64.h"
#include "spdif.h"
+#include "s337m.h"
typedef struct WAVDemuxContext {
const AVClass *class;
@@ -61,9 +62,11 @@ typedef struct WAVDemuxContext {
int ignore_length;
int max_size;
int spdif;
+int s337m;
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
+int non_pcm_mode;
} WAVDemuxContext;
#define OFFSET(x) offsetof(WAVDemuxContext, x)
@@ -74,12 +77,18 @@ static const AVOption demux_options[] = {
{ "ignore_length", "Ignore length", OFFSET(ignore_length),
AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, DEC },
#endif
{ "max_size", "max size of single packet", OFFSET(max_size),
AV_OPT_TYPE_INT, { .i64 = 4096 }, 1024, 1 << 22, DEC },
+#if CONFIG_S337M_DEMUXER
+{"non_pcm_mode", "Chooses what to do with s337m", OFFSET(non_pcm_mode),
AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, DEC, "non_pcm_mode"},
+{"copy", "Pass s337m through unchanged", 0, AV_OPT_TYPE_CONST,
{.i64 = 0}, 0, 1, DEC, "non_pcm_mode"},
+{"demux" , "Demux s337m" , 0, AV_OPT_TYPE_CONST,
{.i64 = 1}, 0, 1, DEC, "non_pcm_mode"},
+#endif
{ NULL },
};
-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)
{
enum AVCodecID codec;
int len = 1<<16;
int ret = ffio_ensure_seekback(s->pb, len);
@@ -92,10 +101,24 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext
*wav)
int64_t pos = avio_tell(s->pb);
len = ret = avio_read(s->pb, buf, len);
if (len >= 0) {
-ret = ff_spdif_probe(buf, len, );
-if (ret > AVPROBE_SCORE_EXTENSION) {
-s->streams[0]->codecpar->codec_id = codec;
-wav->spdif = 1;
+if (CONFIG_SPDIF_DEMUXER) {
+ret = ff_spdif_probe(buf, len, );
+if (ret > AVPROBE_SCORE_EXTENSION) {
+par->codec_id = codec;
+wav->spdif = 1;
+}
+}
+if (CONFIG_S337M_DEMUXER && !wav->spdif
+&& (par->codec_id == AV_CODEC_ID_PCM_S16LE ||
par->codec_id == AV_CODEC_ID_PCM_S24LE) && par->ch_layout.nb_channels == 2) {
+ret = ff_s337m_probe(buf, len, ,
par->bits_per_coded_sample);
+if (ret > AVPROBE_SCORE_EXTENSION) {
+if (wav->non_pcm_mode) {
+par->codec_id = codec;
+wav->s337m = 1;
+} else {
+av_log(s, AV_LOG_INFO, "Passing through S337M
data: codec will not be reported\n");
+}
+}
}
}
avio_seek(s->pb, pos, SEEK_SET);
@@ -104,7 +127,7 @@ static void set_spdif(AVFormatContext *s, WAVDemuxContext
*wav)
}
if (ret < 0)
-av_log(s, AV_LOG_WARNING, "Cannot check for SPDIF\n");
+av_log(s, AV_LOG_WARNING, "Cannot check for SPDIF/S337M\n");
}
}
@@ -668,7 +691,7 @@ break_loop:
ff_metadata_conv_ctx(s, NULL, wav_metadata_conv);
ff_metadata_conv_ctx(s, NULL, ff_riff_info_conv);
-set_spdif(s, wav);
+set_spdif_s337m(s, wav);
return 0;
}
@@ -766,6 +789,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 {
size = wav->max_size;
if (st->codecpar->block_align > 1) {
if (size < st->codecpar->block_align)
@@ -774,6 +801,8 @@ smv_out:
}
size = FFMIN(size, left);
ret = av_get_packet(s->pb, pkt, size);
+}
+
if (ret < 0)
return ret;
pkt->stream_index = 0;
@@ -960,7 +989,7 @@ static int w64_read_header(AVFormatContext *s)
avio_seek(pb, data_ofs, SEEK_SET);
-set_spdif(s, wav);
+set_spdif_s337m(s, wav);
