diff mbox

[FFmpeg-devel,1/2] Use QT format for audio sample descriptors depending on stsd version.

Message ID 20180906153544.138864-2-derek.buitenhuis@gmail.com
State Accepted
Commit d2047c14c0789ce3059b840472fb96fd8bd854a9
Headers show

Commit Message

Derek Buitenhuis Sept. 6, 2018, 3:35 p.m. UTC 
From: Justin Ruggles <justin.ruggles@gmail.com>

ISOBMFF does not allow AudioSampleEntryV1 in stsd version 0, so
assume the descriptor format is QTFF SoundDescriptionV1. ISOBMFF does
not define a version 2.

This fixes audio decoding for some MP4 files generated with Apple
tools. The additional fields present in SoundDescriptionV1/V2 need to
be read in order to correctly read additional boxes that contain
information required for decoding the stream.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/isom.h | 1 +
 libavformat/mov.c  | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Carl Eugen Hoyos Sept. 6, 2018, 4:13 p.m. UTC | #1 
2018-09-06 17:35 GMT+02:00, Derek Buitenhuis <derek.buitenhuis@gmail.com>:
> From: Justin Ruggles <justin.ruggles@gmail.com>
>
> ISOBMFF does not allow AudioSampleEntryV1 in stsd version 0, so
> assume the descriptor format is QTFF SoundDescriptionV1. ISOBMFF does
> not define a version 2.
>
> This fixes audio decoding for some MP4 files generated with Apple
> tools. The additional fields present in SoundDescriptionV1/V2 need to
> be read in order to correctly read additional boxes that contain
> information required for decoding the stream.

Please mention ticket #7376 (and the Handbrake issue) in the commit
message.

Thank you, Carl Eugen
Derek Buitenhuis Sept. 6, 2018, 4:18 p.m. UTC | #2 
On 06/09/2018 17:13, Carl Eugen Hoyos wrote:
> Please mention ticket #7376 (and the Handbrake issue) in the commit
> message.

Wasn't aware there were tickets already, woops.

Added locally.

- Derek
John Stebbins Sept. 6, 2018, 5:39 p.m. UTC | #3 
On 09/06/2018 09:18 AM, Derek Buitenhuis wrote:
> On 06/09/2018 17:13, Carl Eugen Hoyos wrote:
>> Please mention ticket #7376 (and the Handbrake issue) in the commit
>> message.
> Wasn't aware there were tickets already, woops.
>
> Added locally.
>
>

Looks like this also supersedes my earlier patch to fix the same problem... your solution is better, thanks ;)

https://ffmpeg.org/pipermail/ffmpeg-devel/2018-August/233733.html
diff mbox

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 51abea52de..89659db8ee 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -217,6 +217,7 @@  typedef struct MOVStreamContext {
     int *extradata_size;
     int last_stsd_index;
     int stsd_count;
+    int stsd_version;
 
     int32_t *display_matrix;
     AVStereo3D *stereo3d;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index d922e0f173..ea99616dc8 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2120,8 +2120,8 @@  static void mov_parse_stsd_audio(MOVContext *c, AVIOContext *pb,
     // Read QT version 1 fields. In version 0 these do not exist.
     av_log(c->fc, AV_LOG_TRACE, "version =%d, isom =%d\n", version, c->isom);
     if (!c->isom ||
-        (compatible_brands && strstr(compatible_brands->value, "qt  "))) {
-
+        (compatible_brands && strstr(compatible_brands->value, "qt  ")) ||
+        (sc->stsd_version == 0 && version > 0)) {
         if (version == 1) {
             sc->samples_per_frame = avio_rb32(pb);
             avio_rb32(pb); /* bytes per packet */
@@ -2554,7 +2554,7 @@  static int mov_read_stsd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     st = c->fc->streams[c->fc->nb_streams - 1];
     sc = st->priv_data;
 
-    avio_r8(pb); /* version */
+    sc->stsd_version = avio_r8(pb);
     avio_rb24(pb); /* flags */
     entries = avio_rb32(pb);