[FFmpeg-devel] [PATCH 0/6] wavdev: s337m support

[FFmpeg-devel] [PATCH 0/6] wavdev: s337m support

[Nicolas Gaullier]

This is the follow up of a past work
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=2593

In my understanding, the code review was running pretty well,
but there was two issues:
- the first one was fixed with the creation of the dolby_e parser
which removed the need of one of the initial patches
- the second one is much embarrassing for me, it is the need
to be able to pass-through s337m (as in current code).
This is typically required to remux s337m to mxf, as there is no s337m submuxer available yet.

And the scope of this option is not clear to me.
I have understood that by default, the s337m demux shall be enabled and I changed my code accordingly.
I also understand we cannot multiply options for every little thing, so I proposed a global option:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20211012154156.1726-2-nicolas.gaullier@cji.paris/
Indeed, it is interesting, for example when using ffprobe as a light "QC" tool and to reject for example
mpegts files that do not have any single PMT, etc.
But I had no answer despite several pings, so I suspect it is not the way to do.

Here, I lastly propose to 'reuse' the AVOption used for s302m as the problem is really similar.
At the end, I find this pretty cool like this... Hope you will like it too!
Anyway, please give me a feedback.

Sample file used for fate:
https://0x0.st/zdW-.wav

Nicolas Gaullier (6):
  avformat/s337m: Split read_packet/get_packet
  avformat/s337m: Consider container bit resolution
  avformat/s337m: New ff_s337m_probe()
  avformat/wavdec: s337m support
  avformat/wavdec.c: Reindent after last commit
  avformat/wavdec: Test s337m

 libavformat/s337m.c      | 71 ++++++++++++++++++++++++++++++++++++----
 libavformat/s337m.h      | 54 ++++++++++++++++++++++++++++++
 libavformat/wavdec.c     | 61 +++++++++++++++++++++++++---------
 tests/Makefile           |  1 +
 tests/fate/audio.mak     |  3 ++
 tests/ref/fate/s337m-wav | 10 ++++++
 6 files changed, 177 insertions(+), 23 deletions(-)
 create mode 100644 libavformat/s337m.h
 create mode 100644 tests/ref/fate/s337m-wav

-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/6] avformat/s337m: Split read_packet/get_packet

[Nicolas Gaullier]

Prepare use of s337m_get_packet from outside.
---
 libavformat/s337m.c | 24 ++++++++++++++++++++----
 libavformat/s337m.h | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 libavformat/s337m.h

diff --git a/libavformat/s337m.c b/libavformat/s337m.c
index 6fecfeffae..582c8b3670 100644
--- a/libavformat/s337m.c
+++ b/libavformat/s337m.c
@@ -22,6 +22,7 @@
 #include "avformat.h"
 #include "internal.h"
 #include "spdif.h"
+#include "s337m.h"
 
 #define MARKER_16LE         0x72F81F4E
 #define MARKER_20LE         0x20876FF0E154
@@ -142,17 +143,20 @@ static void bswap_buf24(uint8_t *data, int size)
         FFSWAP(uint8_t, data[0], data[2]);
 }
 
-static int s337m_read_packet(AVFormatContext *s, AVPacket *pkt)
+int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc)
 {
-    AVIOContext *pb = s->pb;
     uint64_t state = 0;
     int ret, data_type, data_size, offset;
-    enum AVCodecID codec;
+    int64_t orig_pos = avio_tell(pb);
 
     while (!IS_LE_MARKER(state)) {
         state = (state << 8) | avio_r8(pb);
         if (avio_feof(pb))
             return AVERROR_EOF;
+        if (avio_tell(pb) - orig_pos + 6 >= size) {
+            av_log(avc, AV_LOG_ERROR, "s337m : sync bytes not found at packet pos=0x%"PRIx64" size=%d\n", orig_pos, size);
+            return AVERROR_INVALIDDATA;
+        }
     }
 
     if (IS_16LE_MARKER(state)) {
@@ -163,8 +167,9 @@ static int s337m_read_packet(AVFormatContext *s, AVPacket *pkt)
         data_size = avio_rl24(pb);
     }
 
-    if ((ret = s337m_get_offset_and_codec(s, state, data_type, data_size, &offset, &codec)) < 0)
+    if ((ret = s337m_get_offset_and_codec(avc, state, data_type, data_size, &offset, codec)) < 0)
         return ret;
+    offset = FFMIN(offset, size - (avio_tell(pb) - orig_pos));
 
     if ((ret = av_get_packet(pb, pkt, offset)) != offset)
         return ret < 0 ? ret : AVERROR_EOF;
@@ -174,6 +179,17 @@ static int s337m_read_packet(AVFormatContext *s, AVPacket *pkt)
     else
         bswap_buf24(pkt->data, pkt->size);
 
+    return 0;
+}
+
+static int s337m_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    enum AVCodecID codec;
+    int ret;
+
+    if ((ret = ff_s337m_get_packet(s->pb, pkt, avio_size(s->pb), &codec, s)) < 0)
+        return ret;
+
     if (!s->nb_streams) {
         AVStream *st = avformat_new_stream(s, NULL);
         if (!st) {
diff --git a/libavformat/s337m.h b/libavformat/s337m.h
new file mode 100644
index 0000000000..f7bd0c16f6
--- /dev/null
+++ b/libavformat/s337m.h
@@ -0,0 +1,37 @@
+/*
+ * SMPTE ST 337 common header
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFORMAT_S337M_H
+#define AVFORMAT_S337M_H
+
+/**
+ * Read s337m packets in a PCM_S16LE/S24LE stereo stream
+ * Returns the first inner packet found
+ * Note that it does not require a clean guard band
+ * @param pb Associated IO context
+ * @param pkt On success, returns a DOLBY E packet
+ * @param size Maximum IO read size available for reading at current position
+ * @param codec Returns AV_CODEC_ID_DOLBY_E
+ * @param avc For av_log
+ * @return = 0 on success (an error is raised if no s337m was found)
+ */
+int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc);
+
+#endif /* AVFORMAT_S337M_H */
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Nicolas Gaullier]

Prepare the support of s337m in muxers other than raw (ex: wav).
For example, this forbids reading 16 bits DolbyE stream from a 24 bit wav file.
---
 libavformat/s337m.c | 21 +++++++++++++++------
 libavformat/s337m.h |  3 ++-
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/libavformat/s337m.c b/libavformat/s337m.c
index 582c8b3670..2a566e3cba 100644
--- a/libavformat/s337m.c
+++ b/libavformat/s337m.c
@@ -35,7 +35,7 @@
 
 static int s337m_get_offset_and_codec(void *avc,
                                       uint64_t state,
-                                      int data_type, int data_size,
+                                      int data_type, int data_size, int container_word_bits,
                                       int *offset, enum AVCodecID *codec)
 {
     int word_bits;
@@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void *avc,
             avpriv_report_missing_feature(avc, "Data type %#x in SMPTE 337M", data_type & 0x1F);
         return AVERROR_PATCHWELCOME;
     }
+    if (container_word_bits &&
+        !(container_word_bits == 16 && word_bits == 16) &&
+        !(container_word_bits == 24 && word_bits == 20) &&
+        !(container_word_bits == 24 && word_bits == 24)) {
+        return AVERROR_INVALIDDATA;
+    }
 
     if (codec)
         *codec = AV_CODEC_ID_DOLBY_E;
@@ -105,7 +111,7 @@ static int s337m_probe(const AVProbeData *p)
             data_size = AV_RL24(buf + 3);
         }
 
-        if (s337m_get_offset_and_codec(NULL, state, data_type, data_size, &offset, NULL))
+        if (s337m_get_offset_and_codec(NULL, state, data_type, data_size, 0, &offset, NULL))
             continue;
 
         i = IS_16LE_MARKER(state) ? 0 : IS_20LE_MARKER(state) ? 1 : 2;
@@ -143,13 +149,16 @@ static void bswap_buf24(uint8_t *data, int size)
         FFSWAP(uint8_t, data[0], data[2]);
 }
 
-int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc)
+int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc, int container_word_bits)
 {
     uint64_t state = 0;
     int ret, data_type, data_size, offset;
     int64_t orig_pos = avio_tell(pb);
 
-    while (!IS_LE_MARKER(state)) {
+    if (container_word_bits && container_word_bits != 16 && container_word_bits != 24)
+        return AVERROR_INVALIDDATA;
+    while ((container_word_bits == 24 || !IS_16LE_MARKER(state))
+        && (container_word_bits == 16 || !IS_20LE_MARKER(state) && !IS_24LE_MARKER(state))) {
         state = (state << 8) | avio_r8(pb);
         if (avio_feof(pb))
             return AVERROR_EOF;
@@ -167,7 +176,7 @@ int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID
         data_size = avio_rl24(pb);
     }
 
-    if ((ret = s337m_get_offset_and_codec(avc, state, data_type, data_size, &offset, codec)) < 0)
+    if ((ret = s337m_get_offset_and_codec(avc, state, data_type, data_size, container_word_bits, &offset, codec)) < 0)
         return ret;
     offset = FFMIN(offset, size - (avio_tell(pb) - orig_pos));
 
@@ -187,7 +196,7 @@ static int s337m_read_packet(AVFormatContext *s, AVPacket *pkt)
     enum AVCodecID codec;
     int ret;
 
-    if ((ret = ff_s337m_get_packet(s->pb, pkt, avio_size(s->pb), &codec, s)) < 0)
+    if ((ret = ff_s337m_get_packet(s->pb, pkt, avio_size(s->pb), &codec, s, 0)) < 0)
         return ret;
 
     if (!s->nb_streams) {
diff --git a/libavformat/s337m.h b/libavformat/s337m.h
index f7bd0c16f6..af2c4c85a3 100644
--- a/libavformat/s337m.h
+++ b/libavformat/s337m.h
@@ -30,8 +30,9 @@
  * @param size Maximum IO read size available for reading at current position
  * @param codec Returns AV_CODEC_ID_DOLBY_E
  * @param avc For av_log
+ * @param container_word_bits 16,24, or 0 for autodetect
  * @return = 0 on success (an error is raised if no s337m was found)
  */
-int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc);
+int ff_s337m_get_packet(AVIOContext *pb, AVPacket *pkt, int size, enum AVCodecID *codec, void *avc, int container_word_bits);
 
 #endif /* AVFORMAT_S337M_H */
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/6] avformat/s337m: New ff_s337m_probe()

[Nicolas Gaullier]

Similar to ff_spdif_probe() with just an additional checking of
the bit resolution of the container as it may be 16 or 24 for s337m.
---
 libavformat/s337m.c | 32 ++++++++++++++++++++++++++++++++
 libavformat/s337m.h | 16 ++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/libavformat/s337m.c b/libavformat/s337m.c
index 2a566e3cba..9d2ac265b6 100644
--- a/libavformat/s337m.c
+++ b/libavformat/s337m.c
@@ -135,6 +135,38 @@ static int s337m_probe(const AVProbeData *p)
     return 0;
 }
 
+int ff_s337m_probe(const uint8_t *buf, int size, enum AVCodecID *codec, int container_word_bits)
+{
+    int pos = 0;
+    int consecutive_codes = 0;
+
+    if ( size < S337M_MIN_OFFSET)
+        return 0;
+    size = FFMIN(3 * S337M_MAX_OFFSET, size);
+    if (container_word_bits != 16 && container_word_bits != 24)
+        return AVERROR_INVALIDDATA;
+
+    do {
+        uint64_t state;
+        int data_type, data_size, offset;
+        while (pos < size - 12 && !buf[pos]) {
+            pos++;
+        }
+        if (pos >= size - 12 || pos < S337M_PROBE_GUARDBAND_MIN_BYTES || pos % (container_word_bits == 16 ? 4 : 6))
+            return 0;
+        state = container_word_bits == 16 ? AV_RB32(buf + pos) : AV_RB48(buf + pos);
+        if (!IS_LE_MARKER(state))
+            return 0;
+        data_type = container_word_bits == 16 ? AV_RL16(buf + pos + 4) : AV_RL24(buf + pos + 6);
+        data_size = container_word_bits == 16 ? AV_RL16(buf + pos + 6) : AV_RL24(buf + pos + 9);
+        if (s337m_get_offset_and_codec(NULL, state, data_type, data_size, container_word_bits, &offset, codec))
+            return 0;
+        pos = ++consecutive_codes * (offset + 4*(container_word_bits == 16 ? 4 : 6));
+    } while (consecutive_codes < 3);
+
+    return AVPROBE_SCORE_MAX;
+}
+
 static int s337m_read_header(AVFormatContext *s)
 {
     s->ctx_flags |= AVFMTCTX_NOHEADER;
diff --git a/libavformat/s337m.h b/libavformat/s337m.h
index af2c4c85a3..94e79dce5d 100644
--- a/libavformat/s337m.h
+++ b/libavformat/s337m.h
@@ -21,6 +21,22 @@
 #ifndef AVFORMAT_S337M_H
 #define AVFORMAT_S337M_H
 
+#define S337M_MIN_OFFSET 1601*4
+#define S337M_MAX_OFFSET 2002*6
+
+#define S337M_PROBE_GUARDBAND_MIN_BYTES     0
+
+/**
+ * Detect s337m packets in a PCM_S16LE/S24LE stereo stream
+ * Requires 3 samples with enough (S337M_PROBE_GUARDBAND_MIN_BYTES) and clean (set to zero) guard band
+ * @param p_buf Buffer
+ * @param size Buffer size
+ * @param codec Returns AV_CODEC_ID_DOLBY_E upon successful probing
+ * @param container_word_bits 16 or 24
+ * @return = AVPROBE_SCORE
+ */
+int ff_s337m_probe(const uint8_t *p_buf, int size, enum AVCodecID *codec, int container_word_bits);
+
 /**
  * Read s337m packets in a PCM_S16LE/S24LE stereo stream
  * Returns the first inner packet found
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[Nicolas Gaullier]

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, &codec);
-                    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, &codec);
+                        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, &codec, 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);
 
     return 0;
 }
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/6] avformat/wavdec.c: Reindent after last commit

[Nicolas Gaullier]

---
 libavformat/wavdec.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index fd9ca89880..29192e48f0 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -793,14 +793,14 @@ smv_out:
         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)
-            size = st->codecpar->block_align;
-        size = (size / st->codecpar->block_align) * st->codecpar->block_align;
-    }
-    size = FFMIN(size, left);
-    ret  = av_get_packet(s->pb, pkt, size);
+        size = wav->max_size;
+        if (st->codecpar->block_align > 1) {
+            if (size < st->codecpar->block_align)
+                size = st->codecpar->block_align;
+            size = (size / st->codecpar->block_align) * st->codecpar->block_align;
+        }
+        size = FFMIN(size, left);
+        ret  = av_get_packet(s->pb, pkt, size);
     }
 
     if (ret < 0)
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/6] avformat/wavdec: Test s337m

[Nicolas Gaullier]

Test s337m probing in wav container.
Test dolby_e demuxing for 20 bits with program config '5.1+2'.
---
 tests/Makefile           |  1 +
 tests/fate/audio.mak     |  3 +++
 tests/ref/fate/s337m-wav | 10 ++++++++++
 3 files changed, 14 insertions(+)
 create mode 100644 tests/ref/fate/s337m-wav

diff --git a/tests/Makefile b/tests/Makefile
index 1d50e1d175..d78f78d1b8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -99,6 +99,7 @@ REMUX = $(call ALLYES, $(firstword $(1))_MUXER $(lastword $(1))_DEMUXER \
                        $(2) FILE_PROTOCOL PIPE_PROTOCOL FRAMECRC_MUXER)
 
 DEMDEC  = $(call ALLYES, $(1)_DEMUXER $(2:%=%_DECODER) $(3) FILE_PROTOCOL)
+DEMDEMDEC = $(call ALLYES, $(1)_DEMUXER $(2)_DEMUXER $(3:%=%_DECODER))
 ENCMUX  = $(call ALLYES, $(1:%=%_ENCODER) $(2)_MUXER   $(3))
 
 FRAMEMD5 = $(call ALLYES, $(1)_DEMUXER $(2:%=%_DECODER) $(3)                  \
diff --git a/tests/fate/audio.mak b/tests/fate/audio.mak
index 65317c8d45..1d1c75b0b3 100644
--- a/tests/fate/audio.mak
+++ b/tests/fate/audio.mak
@@ -31,6 +31,9 @@ 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-s337m-wav
+fate-s337m-wav: CMD = framecrc -i $(TARGET_SAMPLES)/dolby_e/512.wav -vn -c:a copy
+
 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 -af aresample
 fate-dss-sp: CMD = framecrc -i $(TARGET_SAMPLES)/dss/sp.dss -frames 30
diff --git a/tests/ref/fate/s337m-wav b/tests/ref/fate/s337m-wav
new file mode 100644
index 0000000000..768a6f0161
--- /dev/null
+++ b/tests/ref/fate/s337m-wav
@@ -0,0 +1,10 @@
+#tb 0: 1/48000
+#media_type 0: audio
+#codec_id 0: dolby_e
+#sample_rate 0: 44800
+#channel_layout_name 0: 7.1
+0,          0,          0,     1920,    11496, 0x05a9c147
+0,       1920,       1920,     1920,    11496, 0x1d44d2b4
+0,       3840,       3840,     1920,    11496, 0x4e078953
+0,       5760,       5760,     1920,    11496, 0x1c73b1a1
+0,       7680,       7680,     1920,    11262, 0xfa179fc8
-- 
2.30.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avformat/s337m: Split read_packet/get_packet

[Tomas Härdin]

mån 2023-02-13 klockan 19:09 +0100 skrev Nicolas Gaullier:
> Prepare use of s337m_get_packet from outside.
> ---
>  libavformat/s337m.c | 24 ++++++++++++++++++++----
>  libavformat/s337m.h | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+), 4 deletions(-)
>  create mode 100644 libavformat/s337m.h

Looks OK

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Tomas Härdin]

mån 2023-02-13 klockan 19:09 +0100 skrev Nicolas Gaullier:
> Prepare the support of s337m in muxers other than raw (ex: wav).
> For example, this forbids reading 16 bits DolbyE stream from a 24 bit
> wav file.
> ---
>  libavformat/s337m.c | 21 +++++++++++++++------
>  libavformat/s337m.h |  3 ++-
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/s337m.c b/libavformat/s337m.c
> index 582c8b3670..2a566e3cba 100644
> --- a/libavformat/s337m.c
> +++ b/libavformat/s337m.c
> @@ -35,7 +35,7 @@
>  
>  static int s337m_get_offset_and_codec(void *avc,
>                                        uint64_t state,
> -                                      int data_type, int data_size,
> +                                      int data_type, int data_size,
> int container_word_bits,
>                                        int *offset, enum AVCodecID
> *codec)
>  {
>      int word_bits;
> @@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void *avc,
>              avpriv_report_missing_feature(avc, "Data type %#x in
> SMPTE 337M", data_type & 0x1F);
>          return AVERROR_PATCHWELCOME;
>      }
> +    if (container_word_bits &&
> +        !(container_word_bits == 16 && word_bits == 16) &&
> +        !(container_word_bits == 24 && word_bits == 20) &&

I presume 20/24 is intentional. Does WAV not support signalling 20-bit?

The rest looks OK enough

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avformat/s337m: New ff_s337m_probe()

[Tomas Härdin]

mån 2023-02-13 klockan 19:09 +0100 skrev Nicolas Gaullier:
> +    do {
> +        uint64_t state;
> +        int data_type, data_size, offset;
> +        while (pos < size - 12 && !buf[pos]) {
> +            pos++;
> +        }
> +        if (pos >= size - 12 || pos <
> S337M_PROBE_GUARDBAND_MIN_BYTES || pos % (container_word_bits == 16 ?
> 4 : 6))
> +            return 0;
> +        state = container_word_bits == 16 ? AV_RB32(buf + pos) :
> AV_RB48(buf + pos);
> +        if (!IS_LE_MARKER(state))
> +            return 0;
> +        data_type = container_word_bits == 16 ? AV_RL16(buf + pos +
> 4) : AV_RL24(buf + pos + 6);
> +        data_size = container_word_bits == 16 ? AV_RL16(buf + pos +
> 6) : AV_RL24(buf + pos + 9);
> +        if (s337m_get_offset_and_codec(NULL, state, data_type,
> data_size, container_word_bits, &offset, codec))
> +            return 0;
> +        pos = ++consecutive_codes * (offset + 4*(container_word_bits
> == 16 ? 4 : 6));
> +    } while (consecutive_codes < 3);
> +
> +    return AVPROBE_SCORE_MAX;
> +}

The logic here is a bit hairy and I don't have time atm to digest it,
but is it entirely contained in S337m or would one need to read other
specs too?

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[Tomas Härdin]

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Nicolas Gaullier]

>> @@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void *avc,
>>              avpriv_report_missing_feature(avc, "Data type %#x in 
>> SMPTE 337M", data_type & 0x1F);
>>          return AVERROR_PATCHWELCOME;
>>      }
>> +    if (container_word_bits &&
>> +        !(container_word_bits == 16 && word_bits == 16) &&
>> +        !(container_word_bits == 24 && word_bits == 20) &&
>
>I presume 20/24 is intentional. Does WAV not support signalling 20-bit?
>
>The rest looks OK enough
>
>/Tomas

You are true, I meant "container_word_bits" as "block_size" rather than "valid bits per sample" and
I think this should be clarified is the latter integration code in "wavdec: s337m support" patch where
I use par->bits_per_coded_sample...
But I should have rather coded "AV_CODEC_ID_PCM_S16LE ? 16 : 24"
This is in case a wav file would be detected as 20 bits in a 24 bits container (I don't think it is supported yet, but it could as bitspersample and validbitpersample are two different fields in WAVE_FORMAT_EXTENSIBLE).
Are you ok with this ?

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avformat/s337m: New ff_s337m_probe()

[Nicolas Gaullier]

> The logic here is a bit hairy and I don't have time atm to digest it, but is it entirely contained in S337m or would one need to read other specs too?
>
>/Tomas

ff_s337m_probe is very similar to s337m_probe: what mainly differs is the input parameters.
The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
Currently it is set to 0, so has no effect (and of course I can remove it if someone object).
There is two things to know about it:
- one is that some DolbyE decoder implementations does not support the s337m sync word to be the first word,
A minimal guard band (full of zero) is required in such a case : 1 word is enough in the cases I experimented.
One developer might find it useful to set S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject such files.
- one other thing is that, currently, the detection is based on 3 consecutive samples,
But there are other implementations in the wild. A common single-sample implementation is to simply require
a sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake detection.
(for 16 bits, this is really dangerous!; for 24 bits, I think it is fair but would still require some little additions to be 100% sure).

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[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...
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avformat/s337m: New ff_s337m_probe()

[Tomas Härdin]

fre 2023-02-17 klockan 10:12 +0000 skrev Nicolas Gaullier:
> > The logic here is a bit hairy and I don't have time atm to digest
> > it, but is it entirely contained in S337m or would one need to read
> > other specs too?
> > 
> > /Tomas
> 
> ff_s337m_probe is very similar to s337m_probe: what mainly differs is
> the input parameters.
> The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
> Currently it is set to 0, so has no effect (and of course I can
> remove it if someone object).
> There is two things to know about it:
> - one is that some DolbyE decoder implementations does not support
> the s337m sync word to be the first word,
> A minimal guard band (full of zero) is required in such a case : 1
> word is enough in the cases I experimented.
> One developer might find it useful to set
> S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject
> such files.
> - one other thing is that, currently, the detection is based on 3
> consecutive samples,
> But there are other implementations in the wild. A common single-
> sample implementation is to simply require
> a sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake
> detection.
> (for 16 bits, this is really dangerous!; for 24 bits, I think it is
> fair but would still require some little additions to be 100% sure).

What a mess.. And there's no other way to reliably probe this?

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Tomas Härdin]

fre 2023-02-17 klockan 09:44 +0000 skrev Nicolas Gaullier:
> > > @@ -56,6 +56,12 @@ static int s337m_get_offset_and_codec(void
> > > *avc,
> > >              avpriv_report_missing_feature(avc, "Data type %#x in
> > > SMPTE 337M", data_type & 0x1F);
> > >          return AVERROR_PATCHWELCOME;
> > >      }
> > > +    if (container_word_bits &&
> > > +        !(container_word_bits == 16 && word_bits == 16) &&
> > > +        !(container_word_bits == 24 && word_bits == 20) &&
> > 
> > I presume 20/24 is intentional. Does WAV not support signalling 20-
> > bit?
> > 
> > The rest looks OK enough
> > 
> > /Tomas
> 
> You are true, I meant "container_word_bits" as "block_size" rather
> than "valid bits per sample" and
> I think this should be clarified is the latter integration code in
> "wavdec: s337m support" patch where
> I use par->bits_per_coded_sample...
> But I should have rather coded "AV_CODEC_ID_PCM_S16LE ? 16 : 24"
> This is in case a wav file would be detected as 20 bits in a 24 bits
> container (I don't think it is supported yet, but it could as
> bitspersample and validbitpersample are two different fields in
> WAVE_FORMAT_EXTENSIBLE).
> Are you ok with this ?

I haven't worked enough with S377m to really know, but I do know it's a
mess. Is there a way to differentiate "regular" packed 20-bit audio
from S377m in wav?

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[Tomas Härdin]

fre 2023-02-17 klockan 10:30 +0000 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Nicolas Gaullier]

>I haven't worked enough with S377m to really know, but I do know it's a mess. Is there a way to differentiate "regular" packed 20-bit audio from S377m in wav?
>
>/Tomas

There is a relevant overview of S337m in this dolby paper:
https://developer.dolby.com/globalassets/professional/dolby-e/dolby-e-tech-doc_1.2.pdf
Page 25, one can read:
SMPTE 337M is the primary method by which Dolby E is able to work in existing
facilities and with existing devices. The standard is written such that the same AES3
interface can be shared with the normal PCM audio usage either by treating the AES3
channels independently or by alternating between data and linear PCM usage.
Devices that are specifically designed for SMPTE 337M/Dolby E compatibility
should be able to transition easily between both usages.

The whole design is to not signal, not differentiate "normal PCM audio usage" from s337m.
And indeed, manufacturers have implemented probing in all their hardware/software (be it linear/SDI input, or mxf/wav files input etc.).

Note: ARD/ZDF mxf file format being "the" world exception here, as dolby_e/non-pcm must be signaled, made explicit:
https://www.ard.de/ard/die-ard/ARD-ZDF-HDF02a-AVC-I-100-1080i-25-8-Audio-Tracks-100.pdf

In ffmpeg, we already have spdif probing in wav, so there is nothing really new technically speaking.
Quoting the previous dolby paper, about spdif (IEC61937):
The IEC61937 format is similar to the SMPTE 337M format and can be considered a subset of SMPTE 337M
for some data types (see SMPTE 337M Section 7), however there are significant differences in the two standards.
In particular IEC61937 does not support the Dolby E data type.

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avformat/s337m: New ff_s337m_probe()

[Nicolas Gaullier]

>> ff_s337m_probe is very similar to s337m_probe: what mainly differs is 
>> the input parameters.
>> The one little thing I added is the S337M_PROBE_GUARDBAND_MIN_BYTES.
>> Currently it is set to 0, so has no effect (and of course I can remove 
>> it if someone object).
>> There is two things to know about it:
>> - one is that some DolbyE decoder implementations does not support the 
>> s337m sync word to be the first word, A minimal guard band (full of 
>> zero) is required in such a case : 1 word is enough in the cases I 
>> experimented.
>> One developer might find it useful to set 
>> S337M_PROBE_GUARDBAND_MIN_BYTES to 1 in order to ffprobe-qc/reject 
>> such files.
>> - one other thing is that, currently, the detection is based on 3 
>> consecutive samples, But there are other implementations in the wild. 
>> A common single- sample implementation is to simply require a 
>> sufficient S337M_PROBE_GUARDBAND_MIN_BYTES in order to avoid a fake 
>> detection.
>> (for 16 bits, this is really dangerous!; for 24 bits, I think it is 
>> fair but would still require some little additions to be 100% sure).
>
>What a mess.. And there's no other way to reliably probe this?
>
>/Tomas

No! But the statistics are such that at some time, a fake probe is not "possible".
Anyway, we already have stream probings in current code, not only considering wav.
I think my patch is similar to the spdif probe and safe.

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[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) ?

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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avformat/s337m: Consider container bit resolution

[Tomas Härdin]

tis 2023-02-21 klockan 10:43 +0000 skrev Nicolas Gaullier:
> > I haven't worked enough with S377m to really know, but I do know
> > it's a mess. Is there a way to differentiate "regular" packed 20-
> > bit audio from S377m in wav?
> > 
> > /Tomas
> 
> There is a relevant overview of S337m in this dolby paper:
> https://developer.dolby.com/globalassets/professional/dolby-e/dolby-e-tech-doc_1.2.pdf
> Page 25, one can read:
> SMPTE 337M is the primary method by which Dolby E is able to work in
> existing
> facilities and with existing devices. The standard is written such
> that the same AES3
> interface can be shared with the normal PCM audio usage either by
> treating the AES3
> channels independently or by alternating between data and linear PCM
> usage.
> Devices that are specifically designed for SMPTE 337M/Dolby E
> compatibility
> should be able to transition easily between both usages.
> 
> The whole design is to not signal, not differentiate "normal PCM
> audio usage" from s337m.
> And indeed, manufacturers have implemented probing in all their
> hardware/software (be it linear/SDI input, or mxf/wav files input
> etc.).

Right, it's a deliberate mess.

> Note: ARD/ZDF mxf file format being "the" world exception here, as
> dolby_e/non-pcm must be signaled, made explicit:
>  
> https://www.ard.de/ard/die-ard/ARD-ZDF-HDF02a-AVC-I-100-1080i-25-8-Audio-Tracks-100.pdf

This is the correct approach and precisely what I would expect in the
MXF space. You make it possible to probe in case you have stupid
hardware, but you also explicitly signal it in the container for
systems that are capable of carrying that over.

I will note that Dolby-E support in mxfdec seems to have been in
Baptiste's mind when he first committed the code:

>   //{ {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x02,0x02,0x02,0x03,0x02
> ,0x1C,0x00 }, 15,    AV_CODEC_ID_DOLBY_E }, /* Dolby-E */

Since ccba07d12c. It remains commented out to this day.

Anyway I don't think this has to hold up this patchset. If it works it
works. We can always improve this later if necessary.

/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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] avformat/wavdec: s337m support

[Tomas Härdin]

tis 2023-02-21 klockan 12:30 +0000 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-request@ffmpeg.org with subject "unsubscribe".