Re: [FFmpeg-devel] [PATCH] avformat: add DAT (Digital Audio Tape) demuxer

Re: [FFmpeg-devel] [PATCH] avformat: add DAT (Digital Audio Tape) demuxer

[Jerome Martinez]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

Le 17/01/2025 à 21:43, Michael Niedermayer a écrit :
> On Fri, Jan 17, 2025 at 12:38:02PM +0100, Jerome Martinez wrote:
>> [...]
>> Subject: [PATCH] avformat: add DAT demuxer
> breaks fate-cdxl-pal8-small
>
> I guess the probe function is not working as expected

cdxl parser provides a read_probe value of 1, difficult to compete there...
I added checks on extra metadata (not needed for decoding) for having a 
DAT parser read_probe value of 0 for the cdxl-pal8-small file, it works 
with all the DAT files I found while not changing the fate tests behavior.
Attached is an 2/2 patch to be added to the first one, I separate the 
fix patch from the initial patch in order to keep author name of each patch.

[-- Attachment #2: 0002-avformat-improve-DAT-demuxer-read_probe.patch --]
[-- Type: text/plain, Size: 2482 bytes --]

From 17e4ae0189fc60e4c69357b5324fd84b111a0ac3 Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome@mediaarea.net>
Date: Tue, 21 Jan 2025 22:31:31 +0100
Subject: [PATCH 2/2] avformat: improve DAT demuxer read_probe"

---
 libavformat/dat.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/libavformat/dat.c b/libavformat/dat.c
index 37548a8a73..f7d2de44bc 100644
--- a/libavformat/dat.c
+++ b/libavformat/dat.c
@@ -39,14 +39,41 @@ static int valid_frame(uint8_t *frame)
     uint8_t *scode = frame+DAT_OFFSET;
     uint8_t *subid = scode+7*8;
     uint8_t *mainid = subid+4;
-    int chan_index = (mainid[0] >> 0) & 0x3;
-    int rate_index = (mainid[0] >> 2) & 0x3;
-    int enc_index  = (mainid[1] >> 6) & 0x3;
-    int dataid     = (subid[0] >> 0) & 0xf;
-
-    if (dataid != 0 || encoded_codec[enc_index] == AV_CODEC_ID_NONE ||
+    int pno = 0;
+    int subcode_parity_error = 0;
+    for (int i = 0; i < 7; i++) {
+        uint8_t *subcode = scode+i*8;
+        uint8_t subcode_parity = 0;
+        for (int j = 0; j < 8; j++)
+            subcode_parity ^= subcode[j];
+        if (subcode_parity)
+            subcode_parity_error = 1;
+    }
+    int dataid              = ( subid[0] >> 0) & 0xf;
+    int numpacks            = ( subid[1] >> 0) & 0xf;
+    int pno1                = ( subid[1] >> 4) & 0xf;
+    int pno3                = ( subid[2] >> 0) & 0xf;
+    int pno2                = ( subid[2] >> 4) & 0xf;
+    int chan_index          = (mainid[0] >> 0) & 0x3;
+    int rate_index          = (mainid[0] >> 2) & 0x3;
+    int emphasis_reserved   = (mainid[0] >> 5) & 0x1;
+    int fmtid               = (mainid[0] >> 6) & 0x3;
+    int trackpitch_reserved = (mainid[1] >> 3) & 0x1;
+    int copy_reserved       = (mainid[1] >> 4) & 0x1;
+    int enc_index           = (mainid[1] >> 6) & 0x3;
+    pno = (pno1 << 8) | (pno2 << 4) | pno3;
+
+    if (subcode_parity_error != 0 ||
+        dataid != 0 ||
         encoded_chans[chan_index] == 0 ||
-        encoded_rate[rate_index] == 0)
+        numpacks > 7 ||
+        !(pno == 0xAA || pno == 0xBB || pno == 0xEE || (pno != 0 && pno1 <= 7 && pno2 <= 9 && pno3 <= 9)) ||
+        encoded_rate[rate_index] == 0 ||
+        emphasis_reserved != 0 ||
+        fmtid != 0 ||
+        copy_reserved != 0 ||
+        trackpitch_reserved != 0 ||
+        encoded_codec[enc_index] == AV_CODEC_ID_NONE)
         return 0;
 
     return 1;
-- 
2.46.0.windows.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] avformat: add DAT (Digital Audio Tape) demuxer

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1773 bytes --]

On Wed, Jan 22, 2025 at 12:26:17PM +0100, Jerome Martinez wrote:
> Le 17/01/2025 à 21:43, Michael Niedermayer a écrit :
> > On Fri, Jan 17, 2025 at 12:38:02PM +0100, Jerome Martinez wrote:
> > > [...]
> > > Subject: [PATCH] avformat: add DAT demuxer
> > breaks fate-cdxl-pal8-small
> > 
> > I guess the probe function is not working as expected
> 
> cdxl parser provides a read_probe value of 1, difficult to compete there...
> I added checks on extra metadata (not needed for decoding) for having a DAT
> parser read_probe value of 0 for the cdxl-pal8-small file, it works with all
> the DAT files I found while not changing the fate tests behavior.
> Attached is an 2/2 patch to be added to the first one, I separate the fix
> patch from the initial patch in order to keep author name of each patch.

>  dat.c |   41 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 9dbe3c927caa58e9dd3b9b78fb2ca478380b5f98  0002-avformat-improve-DAT-demuxer-read_probe.patch
> From 17e4ae0189fc60e4c69357b5324fd84b111a0ac3 Mon Sep 17 00:00:00 2001
> From: Jerome Martinez <jerome@mediaarea.net>
> Date: Tue, 21 Jan 2025 22:31:31 +0100
> Subject: [PATCH 2/2] avformat: improve DAT demuxer read_probe"

this passes tests.

but if you want, you could instead of testing "extra metadata (not needed for decoding)"
test more than 1 packet

having a best case score of 1 seems to be something that will
likely fail sooner or later by not detecting a dat file

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The difference between a dictatorship and a democracy is that every 4 years
the population together is allowed to provide 1 bit of input to the government.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] avformat: add DAT (Digital Audio Tape) demuxer

[Jerome Martinez]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

Le 25/01/2025 à 01:46, Michael Niedermayer a écrit :
> [...]
> this passes tests.
>
> but if you want, you could instead of testing "extra metadata (not needed for decoding)"
> test more than 1 packet
>
> having a best case score of 1 seems to be something that will
> likely fail sooner or later by not detecting a dat file

Maybe misunderstanding, the cdxl parser provides score of 1, the DAT 
patches provide a score of the count of detected frames so better.
But attached is a v2 of the 2nd patch, less compilation warnings, using 
less metadata not impacting the decoding and handling corner cases like 
bad metadata e.g. 16-bit 4-ch (impossible) and 32 kHz 12-bit 4-ch.
12-bit is not yet handled but the patch provides the detection of such 
file and a smooth rejection of the file.

Jérôme

[-- Attachment #2: 0002-avformat-dat-improve-DAT-demuxer.patch --]
[-- Type: text/plain, Size: 4208 bytes --]

From 90211198a936ca7087dbf04e5d636fd9992a8332 Mon Sep 17 00:00:00 2001
From: Jerome Martinez <jerome@mediaarea.net>
Date: Wed, 22 Jan 2025 16:08:18 +0100
Subject: [PATCH 2/4] avformat/dat: improve DAT demuxer

Less false positive detection
Better computation of data size with 12-bit
---
 libavformat/dat.c | 49 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/libavformat/dat.c b/libavformat/dat.c
index 37548a8a73..071af9a12a 100644
--- a/libavformat/dat.c
+++ b/libavformat/dat.c
@@ -26,8 +26,9 @@
 #define DAT_PACKET_SIZE 5822
 #define DAT_OFFSET 5760
 
-static const uint32_t encoded_rate[] = { 48000, 44100, 32000, 0 };
-static const uint16_t encoded_size[] = { 5760, 5292, 3840, 0 };
+static const uint16_t encoded_samples[] = { 1440, 1323, 960, 0 };
+static const uint8_t encoded_samples_mul[] = { 1, 2, 0, 0 };
+static const uint8_t encoded_quantization[] = { 16, 12, 0, 0 };
 static const uint8_t encoded_chans[] = { 2, 4, 0, 0 };
 static const enum AVCodecID encoded_codec[] = {
     AV_CODEC_ID_PCM_S16LE,
@@ -41,12 +42,26 @@ static int valid_frame(uint8_t *frame)
     uint8_t *mainid = subid+4;
     int chan_index = (mainid[0] >> 0) & 0x3;
     int rate_index = (mainid[0] >> 2) & 0x3;
+    int fmtid      = (mainid[0] >> 6) & 0x3;
+    int trackpitch = (mainid[1] >> 2) & 0x3;
     int enc_index  = (mainid[1] >> 6) & 0x3;
     int dataid     = (subid[0] >> 0) & 0xf;
-
-    if (dataid != 0 || encoded_codec[enc_index] == AV_CODEC_ID_NONE ||
+    int numpacks   = (subid[1] >> 0) & 0xf;
+    int pno1       = (subid[1] >> 4) & 0xf;
+    int pno3       = (subid[2] >> 0) & 0xf;
+    int pno2       = (subid[2] >> 4) & 0xf;
+    int pno = (pno1 << 8) | (pno2 << 4) | pno3;
+    int encoded_size = encoded_samples[rate_index] * encoded_samples_mul[trackpitch] * encoded_chans[chan_index] * encoded_quantization[enc_index] / 8;
+  
+    if (dataid != 0 ||
+        numpacks > 7 ||
+        pno == 0 ||
         encoded_chans[chan_index] == 0 ||
-        encoded_rate[rate_index] == 0)
+        encoded_samples[rate_index] == 0 ||
+        fmtid != 0 ||
+        encoded_samples_mul[trackpitch] == 0 ||
+        encoded_quantization[enc_index] == 0 ||
+        encoded_size > DAT_OFFSET)
         return 0;
 
     return 1;
@@ -62,7 +77,7 @@ static int read_probe(const AVProbeData *p)
 
         score += ret;
         if (ret == 0)
-            break;
+            return 0;
     }
 
     return FFMIN(score, AVPROBE_SCORE_MAX);
@@ -82,21 +97,29 @@ static int parse_frame(uint8_t *frame, AVCodecParameters *par)
     uint8_t *mainid = subid+4;
     int chan_index = (mainid[0] >> 0) & 0x3;
     int rate_index = (mainid[0] >> 2) & 0x3;
+    int fmtid      = (mainid[0] >> 6) & 0x3;
+    int trackpitch = (mainid[1] >> 2) & 0x3;
     int enc_index  = (mainid[1] >> 6) & 0x3;
     int dataid     = (subid[0] >> 0) & 0xf;
+    int encoded_size = 0;
+    int encoded_size = encoded_samples[rate_index] * encoded_samples_mul[trackpitch] * encoded_chans[chan_index] * encoded_quantization[enc_index] / 8;
 
     par->codec_type = AVMEDIA_TYPE_AUDIO;
     par->codec_id = encoded_codec[enc_index];
     av_channel_layout_default(&par->ch_layout, encoded_chans[chan_index]);
-    par->sample_rate = encoded_rate[rate_index];
-    par->bit_rate = (8LL * DAT_PACKET_SIZE * par->sample_rate) / FFMAX(1, av_get_audio_frame_duration2(par, encoded_size[rate_index]));
-
-    if (dataid != 0 || par->codec_id == AV_CODEC_ID_NONE ||
-        par->ch_layout.nb_channels <= 0 ||
-        par->sample_rate <= 0)
+    par->sample_rate = encoded_samples[rate_index] * 100 / 3;
+    par->bit_rate = (8LL * DAT_PACKET_SIZE * par->sample_rate) / FFMAX(1, av_get_audio_frame_duration2(par, encoded_size));
+
+    if (dataid != 0 ||
+        par->ch_layout.nb_channels == 0 ||
+        par->sample_rate == 0 ||
+        fmtid != 0 ||
+        encoded_samples_mul[trackpitch] == 0 ||
+        encoded_quantization[enc_index] == 0 ||
+        encoded_size > DAT_OFFSET)
         return AVERROR_INVALIDDATA;
 
-    return encoded_size[rate_index];
+    return encoded_size;
 }
 
 static int read_packet(AVFormatContext *s, AVPacket *pkt)
-- 
2.46.0.windows.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] avformat: add DAT (Digital Audio Tape) demuxer

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2551 bytes --]

Hi Jerome

On Wed, Jan 29, 2025 at 06:02:57PM +0100, Jerome Martinez wrote:
> Le 25/01/2025 à 01:46, Michael Niedermayer a écrit :
> > [...]
> > this passes tests.
> > 
> > but if you want, you could instead of testing "extra metadata (not needed for decoding)"
> > test more than 1 packet
> > 
> > having a best case score of 1 seems to be something that will
> > likely fail sooner or later by not detecting a dat file
> 
> Maybe misunderstanding, the cdxl parser provides score of 1, the DAT patches
> provide a score of the count of detected frames so better.

yes, this is the 2nd such dumb mistake i make in the last few days
the whole (still ongoing) "mobbing" compaign against me seems to
affect the quality of my reviews and work


> But attached is a v2 of the 2nd patch, less compilation warnings, using less
> metadata not impacting the decoding and handling corner cases like bad
> metadata e.g. 16-bit 4-ch (impossible) and 32 kHz 12-bit 4-ch.
> 12-bit is not yet handled but the patch provides the detection of such file
> and a smooth rejection of the file.
> 
> Jérôme

>  dat.c |   49 ++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 13 deletions(-)
> cf8d17c26eddaa608bcaa8551c263e6efb077f3d  0002-avformat-dat-improve-DAT-demuxer.patch
> From 90211198a936ca7087dbf04e5d636fd9992a8332 Mon Sep 17 00:00:00 2001
> From: Jerome Martinez <jerome@mediaarea.net>
> Date: Wed, 22 Jan 2025 16:08:18 +0100
> Subject: [PATCH 2/4] avformat/dat: improve DAT demuxer
> 
> Less false positive detection
> Better computation of data size with 12-bit

[...]

> @@ -82,21 +97,29 @@ static int parse_frame(uint8_t *frame, AVCodecParameters *par)
>      uint8_t *mainid = subid+4;
>      int chan_index = (mainid[0] >> 0) & 0x3;
>      int rate_index = (mainid[0] >> 2) & 0x3;
> +    int fmtid      = (mainid[0] >> 6) & 0x3;
> +    int trackpitch = (mainid[1] >> 2) & 0x3;
>      int enc_index  = (mainid[1] >> 6) & 0x3;
>      int dataid     = (subid[0] >> 0) & 0xf;

> +    int encoded_size = 0;
> +    int encoded_size = encoded_samples[rate_index] * encoded_samples_mul[trackpitch] * encoded_chans[chan_index] * encoded_quantization[enc_index] / 8;

duplicate variable

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".