[FFmpeg-devel] [PATCH] libavformat/mov: fix multiple stsd handling of files with edit list, fix #6584
From: Jiejun Zhang
When an edit list exists in a MOV file, counting by stscs no longer
works because stscs' order is different from the actual timeline. This
commit adds stsd-change markers to the actual timeline and changes stsd
according to these markers.
---
libavformat/isom.h | 14 --
libavformat/mov.c | 122 +
2 files changed, 105 insertions(+), 31 deletions(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index ff009b0896..51cd333a8c 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -127,6 +127,11 @@ typedef struct MOVIndexRange {
int64_t end;
} MOVIndexRange;
+typedef struct MOVStsdChangeEntry {
+int64_t sample_index; ///< stsd changes at this sample index
+int stsd_id; ///< stsd changes to stsd_id
+} MOVStsdChangeEntry;
+
typedef struct MOVStreamContext {
AVIOContext *pb;
int pb_is_copied;
@@ -140,16 +145,17 @@ typedef struct MOVStreamContext {
MOVStts *ctts_data;
unsigned int stsc_count;
MOVStsc *stsc_data;
-int stsc_index;
-int stsc_sample;
+MOVStsdChangeEntry *stsd_change_data;
+int stsd_change_count;
+int stsd_change_index;
unsigned int stps_count;
unsigned *stps_data; ///< partial sync sample for mpeg-2 open gop
MOVElst *elst_data;
unsigned int elst_count;
int ctts_index;
int ctts_sample;
-unsigned int sample_size; ///< may contain value calculated from stsd or
value from stsz atom
-unsigned int stsz_sample_size; ///< always contains sample size from stsz
atom
+unsigned int sample_size; ///< may contain value calculated from stsd or
value from stsz atom unsigned
+int stsz_sample_size; ///< always contains sample size from stsz atom
unsigned int sample_count;
int *sample_sizes;
int keyframe_absent;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 63f84be782..f3239047c8 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2923,6 +2923,35 @@ static int64_t add_index_entry(AVStream *st, int64_t
pos, int64_t timestamp,
return index;
}
+static int add_stsd_change_entry(MOVStreamContext *msc, int64_t sample_index,
int stsd_id,
+unsigned int* allocated_size)
+{
+MOVStsdChangeEntry *entries;
+const size_t min_size_needed = (msc->stsd_change_count + 1) *
sizeof(MOVStsdChangeEntry);
+
+const size_t requested_size =
+min_size_needed > *allocated_size ?
+FFMAX(min_size_needed, 2 * (*allocated_size)) :
+min_size_needed;
+
+if ((unsigned)msc->stsd_change_count + 1 >= UINT_MAX /
sizeof(MOVStsdChangeEntry))
+return -1;
+
+entries = av_fast_realloc(msc->stsd_change_data,
+ allocated_size,
+ requested_size);
+
+if (!entries)
+return -1;
+
+msc->stsd_change_data = entries;
+entries[msc->stsd_change_count].sample_index = sample_index;
+entries[msc->stsd_change_count].stsd_id = stsd_id;
+msc->stsd_change_count++;
+
+return 0;
+}
+
/**
* Rewrite timestamps of index entries in the range [end_index -
frame_duration_buffer_size, end_index)
* by subtracting end_ts successively by the amounts given in
frame_duration_buffer.
@@ -3055,6 +3084,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
int num_discarded_begin = 0;
int first_non_zero_audio_edit = -1;
int packet_skip_samples = 0;
+int stsc_index = 0;
+int stsc_sample = 0;
MOVIndexRange *current_index_range;
int i;
@@ -3091,6 +3122,11 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
start_dts = edit_list_dts_entry_end;
+msc->last_stsd_index = -1;
+unsigned int stsd_change_data_allocated_size = 0;
+msc->stsd_change_data = NULL;
+msc->stsd_change_count = 0;
+
while (get_edit_list_entry(mov, msc, edit_list_index,
&edit_list_media_time,
&edit_list_duration, mov->time_scale)) {
av_log(mov->fc, AV_LOG_DEBUG, "Processing st: %d, edit list %"PRId64"
- media time: %"PRId64", duration: %"PRId64"\n",
@@ -3153,6 +3189,20 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
}
current = e_old + index;
+// adjust stsd index
+int time_sample = 0;
+for (int i = 0; i < msc->stsc_count; i++) {
+int next = time_sample + mov_get_stsc_samples(msc, i);
+if (next > index) {
+stsc_index = i;
+stsc_sample = index - time_sample;
+break;
+}
+time_sample = next;
+}
+
+av_log(mov->fc, AV_LOG_INFO, "stream %d, adjusted stsc_index: %d,
stsc_sample: %d\n", st->index, stsc_index, stsc_sample);
+
ctts_index_old = 0;
ctts_sample_old = 0;
@@ -3265,12 +3315,35 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
}
}
-if (add_index_entry(st, current->pos, edit_list
[FFmpeg-devel] [PATCH] libavcodec: fix field_order labelling
Hello all,
This issue originated in this thread
https://github.com/amiaopensource/vrecord/issues/170. On Field Order, in the
QuickTime specification at
https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html
(and similarly in the Matroska specification which adopted similar language)
it states the following meanings for field order values:
> 9 – B is displayed earliest, T is stored first in the file. 14 – T is
> displayed earliest, B is stored first in the file.
This definition is contradicted by other Apple documentation such as
https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION.
An Apple engineer confirmed that the QuickTime specification’s definitions for
those Field Order values is wrong and does not match Apple’s (of FFmpeg’s)
practice, see
https://github.com/amiaopensource/vrecord/issues/170#issuecomment-321937668.
However I think that some of the commenting in ffmpeg is based upon the
inaccurate definitions from Apple. For instance, in that thread David Singer
confirms:
> Ah, not quite. 1 and 6 are indeed 'planar' (all of one field before all of
> the other). They don't concern us. Both 9 and 14 are stored in spatial order
> (i.e. you could do terrible de-interlacing by simply displaying the buffer as
> a frame), and the 9 or 14 value tells you which field is to be displayed
> first.
>
> 9 – T is earlier than B. 14 – B is earlier than T
mov.c associates AV_FIELD_TB with 9 and AV_FIELD_BT with 14 (similar
associations in matroska.h), but avcodec.h states:
> AV_FIELD_TB, //< Top coded first, bottom displayed first
> AV_FIELD_BT, //< Bottom coded first, bottom displayed first
IMHO in both cases of AV_FIELD_TB and AV_FIELD_BT the coding should be referred
as interleaved rather than ‘bottom coded first’ or ‘top coded first’. In the
case of AV_FIELD_TT and AV_FIELD_BB the fields are stored as planar images
where storage order is notable, but with TB and BT the fields are interleaved.
Also utils.c associates these field order values with the following labels:
> AV_FIELD_TB -> "top coded first (swapped)";
> AV_FIELD_BT -> "bottom coded first (swapped)";
From my reading, I infer that "top coded first (swapped)” means "top coded
first, bottom displayed first”; however in practice from files generated by
QuickTime and FFmpeg files with a value of TB have the top field displayed
first, so I think the labels are swapped. In the patch below I suggest using
“top first (interleaved)” for TB and “bottom first (interleaved)” for BT.
Comments?
From de871b3fa891fa0ae6856461c1f8305cc889cde7 Mon Sep 17 00:00:00 2001
From: Dave Rice
Date: Sat, 12 Aug 2017 12:30:43 -0400
Subject: [PATCH] libavcodec: fix field_order labelling
---
libavcodec/avcodec.h | 4 ++--
libavcodec/utils.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c594993766..37c39072b3 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1726,8 +1726,8 @@ enum AVFieldOrder {
AV_FIELD_PROGRESSIVE,
AV_FIELD_TT, //< Top coded_first, top displayed first
AV_FIELD_BB, //< Bottom coded first, bottom displayed first
-AV_FIELD_TB, //< Top coded first, bottom displayed first
-AV_FIELD_BT, //< Bottom coded first, top displayed first
+AV_FIELD_TB, //< Interleaved coding, top displayed first
+AV_FIELD_BT, //< Interleaved coding, bottom displayed first
};
/**
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1336e921c9..926c964846 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1406,9 +1406,9 @@ void avcodec_string(char *buf, int buf_size,
AVCodecContext *enc, int encode)
else if (enc->field_order == AV_FIELD_BB)
field_order = "bottom first";
else if (enc->field_order == AV_FIELD_TB)
-field_order = "top coded first (swapped)";
+field_order = "top first (interleaved)";
else if (enc->field_order == AV_FIELD_BT)
-field_order = "bottom coded first (swapped)";
+field_order = "bottom first (interleaved)";
av_strlcatf(detail, sizeof(detail), "%s, ", field_order);
}
--
2.13.1
Thanks much,
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/ffv1dec_template: Fix undefined shift
On Fri, Aug 11, 2017 at 11:21:21PM +0200, Michael Niedermayer wrote: > Fixes: runtime error: left shift of negative value -127 > Fixes: 2834/clusterfuzz-testcase-minimized-5988039123795968 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/ffv1dec_template.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) patchset applied [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
