diff mbox series

[FFmpeg-devel] avformat/ac3dec: detect fairplay encryption

Message ID 20200220214353.240335-1-sebh@google.com
State New
Headers show
Series [FFmpeg-devel] avformat/ac3dec: detect fairplay encryption
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Sebastian Hubbard Feb. 20, 2020, 9:43 p.m. UTC
Added id3v2_buf and id3v2_buf_size fields to AVProbeData. When an id3v2
tag is found at the start of probed content, id3v2_buf is filled with
a pointer to the start of the tag bytes, and id3v2_buf_size with the
number of bytes in the tag.

ac3_eac3_probe checks the tag when present for a private frame with an
owner identifier of com.apple.streaming.audioDescription, which
signals that the file is encrypted with an Apple DRM scheme. The CRC
checksums for such files are for the content post-decryption, and with
this change they are skipped when Apple DRM is detected. This enables
ffprobe to detect the stream type, even though it will not be playable.

Signed-off-by: Sebastian Hubbard <sebh@google.com>
---
 libavformat/ac3dec.c   | 22 ++++++++++++++++++++--
 libavformat/avformat.h |  2 ++
 libavformat/format.c   |  6 ++++--
 3 files changed, 26 insertions(+), 4 deletions(-)

Comments

Anton Khirnov March 4, 2020, 11:11 a.m. UTC | #1
Quoting Sebastian Hubbard (2020-02-20 22:43:53)
> Added id3v2_buf and id3v2_buf_size fields to AVProbeData. When an id3v2
> tag is found at the start of probed content, id3v2_buf is filled with
> a pointer to the start of the tag bytes, and id3v2_buf_size with the
> number of bytes in the tag.
> 
> ac3_eac3_probe checks the tag when present for a private frame with an
> owner identifier of com.apple.streaming.audioDescription, which
> signals that the file is encrypted with an Apple DRM scheme. The CRC
> checksums for such files are for the content post-decryption, and with
> this change they are skipped when Apple DRM is detected. This enables
> ffprobe to detect the stream type, even though it will not be playable.
> 
> Signed-off-by: Sebastian Hubbard <sebh@google.com>
> ---
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9b9b634ec3..afd1b67b5c 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -448,6 +448,8 @@ typedef struct AVProbeData {
>      unsigned char *buf; /**< Buffer must have AVPROBE_PADDING_SIZE of extra allocated bytes filled with zero. */
>      int buf_size;       /**< Size of buf except extra allocated bytes */
>      const char *mime_type; /**< mime_type, when known. */
> +    unsigned char *id3v2_buf; /**< Bytes of id3v2 tag if one was found at start of file. */
> +    int id3v2_buf_size; /**< Size of id3v2_buf */

You can't just add fields here, it's an ABI break.
Sebastian Hubbard March 4, 2020, 4:30 p.m. UTC | #2
I knew this was an ABI change and probably wasn't going to be
acceptable, but I decided to share it for consideration anyway.
There's special code in av_probe_input_format3 to skip over id3 tags
for the purposes of format detection, but Apple uses id3 tags to
signal information for HLS audio only tracks. Maybe this change
shouldn't be made, but if it could ever be helpful for another format
prober to know about leading id3 tags this is the only way I could
imagine it.
Carl Eugen Hoyos March 4, 2020, 4:56 p.m. UTC | #3
Am Mi., 4. März 2020 um 17:53 Uhr schrieb Sebastian Hubbard
<sebh-at-google.com@ffmpeg.org>:
>
> I knew this was an ABI change and probably wasn't going to be
> acceptable, but I decided to share it for consideration anyway.
> There's special code in av_probe_input_format3 to skip over id3 tags
> for the purposes of format detection, but Apple uses id3 tags to
> signal information for HLS audio only tracks. Maybe this change
> shouldn't be made, but if it could ever be helpful for another format
> prober to know about leading id3 tags this is the only way I could
> imagine it.

If the additional fields are necessary (I don't know), they could be
added to the end of the struct where they do not break abi.

Carl Eugen
Hendrik Leppkes March 4, 2020, 5:09 p.m. UTC | #4
On Wed, Mar 4, 2020 at 5:57 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> Am Mi., 4. März 2020 um 17:53 Uhr schrieb Sebastian Hubbard
> <sebh-at-google.com@ffmpeg.org>:
> >
> > I knew this was an ABI change and probably wasn't going to be
> > acceptable, but I decided to share it for consideration anyway.
> > There's special code in av_probe_input_format3 to skip over id3 tags
> > for the purposes of format detection, but Apple uses id3 tags to
> > signal information for HLS audio only tracks. Maybe this change
> > shouldn't be made, but if it could ever be helpful for another format
> > prober to know about leading id3 tags this is the only way I could
> > imagine it.
>
> If the additional fields are necessary (I don't know), they could be
> added to the end of the struct where they do not break abi.
>

Unfortunately, thats not true for every struct. In this case, for
example, they are being added at the end, but a caller could still
provide a smaller one  and we would try to read/write beyond the
provided struct.

- Hendrik
James Almer March 4, 2020, 5:49 p.m. UTC | #5
On 3/4/2020 1:56 PM, Carl Eugen Hoyos wrote:
> Am Mi., 4. März 2020 um 17:53 Uhr schrieb Sebastian Hubbard
> <sebh-at-google.com@ffmpeg.org>:
>>
>> I knew this was an ABI change and probably wasn't going to be
>> acceptable, but I decided to share it for consideration anyway.
>> There's special code in av_probe_input_format3 to skip over id3 tags
>> for the purposes of format detection, but Apple uses id3 tags to
>> signal information for HLS audio only tracks. Maybe this change
>> shouldn't be made, but if it could ever be helpful for another format
>> prober to know about leading id3 tags this is the only way I could
>> imagine it.
> 
> If the additional fields are necessary (I don't know), they could be
> added to the end of the struct where they do not break abi.
> 
> Carl Eugen

sizeof(AVProbeData) is part of the ABI because its doxy doesn't strictly
state otherwise. This means we can't add new fields to it, even at the
end, until a major bump. Else we'll be breaking ABI.
Carl Eugen Hoyos March 4, 2020, 6:46 p.m. UTC | #6
Am Mi., 4. März 2020 um 18:57 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 3/4/2020 1:56 PM, Carl Eugen Hoyos wrote:
> > Am Mi., 4. März 2020 um 17:53 Uhr schrieb Sebastian Hubbard
> > <sebh-at-google.com@ffmpeg.org>:
> >>
> >> I knew this was an ABI change and probably wasn't going to be
> >> acceptable, but I decided to share it for consideration anyway.
> >> There's special code in av_probe_input_format3 to skip over id3 tags
> >> for the purposes of format detection, but Apple uses id3 tags to
> >> signal information for HLS audio only tracks. Maybe this change
> >> shouldn't be made, but if it could ever be helpful for another format
> >> prober to know about leading id3 tags this is the only way I could
> >> imagine it.
> >
> > If the additional fields are necessary (I don't know), they could be
> > added to the end of the struct where they do not break abi.
> >
> > Carl Eugen
>
> sizeof(AVProbeData) is part of the ABI because its doxy doesn't strictly
> state otherwise. This means we can't add new fields to it, even at the
> end, until a major bump. Else we'll be breaking ABI.

Thank you both for the explanation!

Carl Eugen
diff mbox series

Patch

diff --git a/libavformat/ac3dec.c b/libavformat/ac3dec.c
index 1f87939388..c29c6fb27b 100644
--- a/libavformat/ac3dec.c
+++ b/libavformat/ac3dec.c
@@ -24,17 +24,35 @@ 
 #include "libavcodec/ac3_parser.h"
 #include "avformat.h"
 #include "rawdec.h"
+#include "id3v2.h"
+#include "avio_internal.h"
 
 static int ac3_eac3_probe(const AVProbeData *p, enum AVCodecID expected_codec_id)
 {
     int max_frames, first_frames = 0, frames;
     const uint8_t *buf, *buf2, *end;
     enum AVCodecID codec_id = AV_CODEC_ID_AC3;
+    AVIOContext ioctx;
+    AVDictionary *metadata = NULL;
+    ID3v2ExtraMeta *id3v2_extra_meta = NULL;
+    int skip_crc = 0;
 
     max_frames = 0;
     buf = p->buf;
     end = buf + p->buf_size;
 
+    if (p->id3v2_buf && p->id3v2_buf_size > 0) {
+        ffio_init_context(&ioctx, p->id3v2_buf, p->id3v2_buf_size, 0, NULL, NULL, NULL, NULL);
+        ff_id3v2_read_dict(&ioctx, &metadata, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta);
+        if (!ff_id3v2_parse_priv_dict(&metadata, &id3v2_extra_meta)) {
+            if (av_dict_get(metadata, "id3v2_priv.com.apple.streaming.audioDescription", NULL, 0)) {
+                skip_crc = 1;
+            }
+        }
+        ff_id3v2_free_extra_meta(&id3v2_extra_meta);
+        av_dict_free(&metadata);
+    }
+
     for(; buf < end; buf++) {
         if(buf > p->buf && !(buf[0] == 0x0B && buf[1] == 0x77)
                         && !(buf[0] == 0x77 && buf[1] == 0x0B) )
@@ -72,10 +90,10 @@  static int ac3_eac3_probe(const AVProbeData *p, enum AVCodecID expected_codec_id
                     buf3[i  ] = buf2[i+1];
                     buf3[i+1] = buf2[i  ];
                 }
-                if (av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf3 + 2, frame_size - 2))
+                if (!skip_crc && av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf3 + 2, frame_size - 2))
                     break;
             } else {
-                if (av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf2 + 2, frame_size - 2))
+                if (!skip_crc && av_crc(av_crc_get_table(AV_CRC_16_ANSI), 0, buf2 + 2, frame_size - 2))
                     break;
             }
             if (bitstream_id > 10)
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 9b9b634ec3..afd1b67b5c 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -448,6 +448,8 @@  typedef struct AVProbeData {
     unsigned char *buf; /**< Buffer must have AVPROBE_PADDING_SIZE of extra allocated bytes filled with zero. */
     int buf_size;       /**< Size of buf except extra allocated bytes */
     const char *mime_type; /**< mime_type, when known. */
+    unsigned char *id3v2_buf; /**< Bytes of id3v2 tag if one was found at start of file. */
+    int id3v2_buf_size; /**< Size of id3v2_buf */
 } AVProbeData;
 
 #define AVPROBE_SCORE_RETRY (AVPROBE_SCORE_MAX/4)
diff --git a/libavformat/format.c b/libavformat/format.c
index c47490c8eb..cd510976a6 100644
--- a/libavformat/format.c
+++ b/libavformat/format.c
@@ -149,8 +149,10 @@  ff_const59 AVInputFormat *av_probe_input_format3(ff_const59 AVProbeData *pd, int
         if (lpd.buf_size > id3len + 16) {
             if (lpd.buf_size < 2LL*id3len + 16)
                 nodat = ID3_ALMOST_GREATER_PROBE;
-            lpd.buf      += id3len;
-            lpd.buf_size -= id3len;
+            lpd.id3v2_buf      = lpd.buf;
+            lpd.id3v2_buf_size = id3len;
+            lpd.buf           += id3len;
+            lpd.buf_size      -= id3len;
         } else if (id3len >= PROBE_BUF_MAX) {
             nodat = ID3_GREATER_MAX_PROBE;
         } else