Message ID | CAEUW-_qfbRFVAXHLSMF5sNDcN5MO-XP-MBxGHtQfpk4=oDMw-w@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/configure | warning | Failed to apply patch |
Matthew Szatmary: > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > --- > Changelog | 1 + > libavcodec/packet.h | 14 ++++++++++++++ > libavformat/mov.c | 17 ++++++++++++++++- > 3 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/Changelog b/Changelog > index c37ffa82e1..2d719dd3b1 100644 > --- a/Changelog > +++ b/Changelog > @@ -9,6 +9,7 @@ version <next>: > - VDPAU accelerated HEVC 10/12bit decoding > - ADPCM IMA Ubisoft APM encoder > - Rayman 2 APM muxer > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > version 4.3: > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index 0a19a0eff3..5faa594cf5 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -290,6 +290,20 @@ enum AVPacketSideDataType { > */ > AV_PKT_DATA_S12M_TIMECODE, > > + /** > + * ISO media file edit list side data packet > + * The structure is repeated for each entry in the edit list > + * The number of entries can be calculated > + * by dividing the packet size by the entry size > + * Each entry is 20 bytes and is laid out as follows: > + * @code > + * s64le duration > + * s64le time > + * float32le rate You are obviously copying the MOVElst structure; yet the rate is a 16.16 fixed point number in the file and not a float, so one should probably use this. > + * @endcode > + */ > + AV_PKT_DATA_EDIT_LIST, > + > /** > * The number of side data types. > * This is not part of the public API/ABI in the sense that it may > diff --git a/libavformat/mov.c b/libavformat/mov.c > index d16840f3df..bb2c940e80 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, > AVIOContext *pb, MOVAtom atom) > av_freep(&sc->keyframes); > av_freep(&sc->stts_data); > av_freep(&sc->stps_data); > - av_freep(&sc->elst_data); This is still needed, namely if an error happens before you can attach the stream side-data. E.g. if an invalid edit list is found and standards compliance is set to strict. Or if av_stream_new_side_data() fails. > av_freep(&sc->rap_group); > > return 0; > @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) > AVStream *st = s->streams[i]; > MOVStreamContext *sc = st->priv_data; > > + if (sc->elst_data) { > + uint8_t *elst_data; > + elst_data = av_stream_new_side_data(st, > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); I wonder whether it would be advantageouos to use av_stream_add_side_data() here. > + > + if (!elst_data) > + goto fail; > + > + for (j = 0; j < sc->elst_count; j++) { > + AV_WB64((elst_data+(j*20)), sc->elst_data[j].duration); > + AV_WB64((elst_data+(j*20)+8), sc->elst_data[j].time); "WB" stands for "Write Big-endian", yet your documentation says that it is supposed to be little-endian. > + AV_WB32((elst_data+(j*20)+16), sc->elst_data[j].rate); > + } > + > + av_freep(&sc->elst_data); > + } > + > switch (st->codecpar->codec_type) { > case AVMEDIA_TYPE_AUDIO: > err = ff_replaygain_export(st, s->metadata); >
On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > > Matthew Szatmary: > > Create and populate AVStream side data packet with contents of ISOBMFF > > edit list entries > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > --- > > Changelog | 1 + > > libavcodec/packet.h | 14 ++++++++++++++ > > libavformat/mov.c | 17 ++++++++++++++++- > > 3 files changed, 31 insertions(+), 1 deletion(-) > > > > diff --git a/Changelog b/Changelog > > index c37ffa82e1..2d719dd3b1 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -9,6 +9,7 @@ version <next>: > > - VDPAU accelerated HEVC 10/12bit decoding > > - ADPCM IMA Ubisoft APM encoder > > - Rayman 2 APM muxer > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > version 4.3: > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > index 0a19a0eff3..5faa594cf5 100644 > > --- a/libavcodec/packet.h > > +++ b/libavcodec/packet.h > > @@ -290,6 +290,20 @@ enum AVPacketSideDataType { > > */ > > AV_PKT_DATA_S12M_TIMECODE, > > > > + /** > > + * ISO media file edit list side data packet > > + * The structure is repeated for each entry in the edit list > > + * The number of entries can be calculated > > + * by dividing the packet size by the entry size > > + * Each entry is 20 bytes and is laid out as follows: > > + * @code > > + * s64le duration > > + * s64le time > > + * float32le rate Good point, I will make that change. > > You are obviously copying the MOVElst structure; yet the rate is a 16.16 > fixed point number in the file and not a float, so one should probably > use this. > > > + * @endcode > > + */ > > + AV_PKT_DATA_EDIT_LIST, > > + > > /** > > * The number of side data types. > > * This is not part of the public API/ABI in the sense that it may > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index d16840f3df..bb2c940e80 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, > > AVIOContext *pb, MOVAtom atom) > > av_freep(&sc->keyframes); > > av_freep(&sc->stts_data); > > av_freep(&sc->stps_data); > > - av_freep(&sc->elst_data); > > This is still needed, namely if an error happens before you can attach > the stream side-data. E.g. if an invalid edit list is found and > standards compliance is set to strict. Or if av_stream_new_side_data() > fails. av_freep(&sc->elst_data); is also called in mov_read_close, So it would only leak if the API was used incorrectly. But that said, I think I can move the logic to mov_read_trak, and make the whole point moot. I was just trying to keep it near the other side_data calls. > > > av_freep(&sc->rap_group); > > > > return 0; > > @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) > > AVStream *st = s->streams[i]; > > MOVStreamContext *sc = st->priv_data; > > > > + if (sc->elst_data) { > > + uint8_t *elst_data; > > + elst_data = av_stream_new_side_data(st, > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > I wonder whether it would be advantageouos to use > av_stream_add_side_data() here. av_stream_new_side_data is just a wrapper for av_malloc + av_stream_add_side_data > > + > > + if (!elst_data) > > + goto fail; > > + > > + for (j = 0; j < sc->elst_count; j++) { > > + AV_WB64((elst_data+(j*20)), sc->elst_data[j].duration); > > + AV_WB64((elst_data+(j*20)+8), sc->elst_data[j].time); > > "WB" stands for "Write Big-endian", yet your documentation says that it > is supposed to be little-endian. thanks, new patch included > > > + AV_WB32((elst_data+(j*20)+16), sc->elst_data[j].rate); > > + } > > + > > + av_freep(&sc->elst_data); > > + } > > + > > switch (st->codecpar->codec_type) { > > case AVMEDIA_TYPE_AUDIO: > > err = ff_replaygain_export(st, s->metadata); > > > > _______________________________________________ > 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". Create and populate AVStream side data packet with contents of ISOBMFF edit list entries Signed-off-by: Matthew Szatmary <matt@szatmary.org> --- Changelog | 1 + libavcodec/packet.h | 15 +++++++++++++++ libavformat/mov.c | 15 +++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/Changelog b/Changelog index 6f648bff2b..d51e836301 100644 --- a/Changelog +++ b/Changelog @@ -10,6 +10,7 @@ version <next>: - ADPCM IMA Ubisoft APM encoder - Rayman 2 APM muxer - AV1 encoding support SVT-AV1 +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data version 4.3: diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 0a19a0eff3..1f0e3405ed 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -290,6 +290,21 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_S12M_TIMECODE, + /** + * ISO media file edit list side data packet + * The structure is repeated for each entry in the edit list + * The number of entries can be calculated + * by dividing the packet size by the entry size + * Each entry is 20 bytes and is laid out as follows: + * @code + * s64be segment duration + * s64be media time + * s16be media rate + * s16be media rate fraction + * @endcode + */ + AV_PKT_DATA_EDIT_LIST, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/mov.c b/libavformat/mov.c index d16840f3df..fbfcdddf3f 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) && sc->time_scale == st->codecpar->sample_rate) { st->need_parsing = AVSTREAM_PARSE_FULL; } + + if (sc->elst_data) { + int i; + uint8_t *elst_data; + elst_data = av_stream_new_side_data(st, AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); + if (elst_data) { + for (i = 0; i < sc->elst_count; i++) { + uint32_t media_rate = (uint32_t)(sc->elst_data[i].rate * 65536.0); + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); + AV_WB32((elst_data+(i*20)+16), media_rate); + } + } + } + /* Do not need those anymore. */ av_freep(&sc->chunk_offsets); av_freep(&sc->sample_sizes);
Create and populate AVStream side data packet with contents of ISOBMFF
edit list entries
Signed-off-by: Matthew Szatmary <matt@szatmary.org>
---
Changelog | 1 +
libavcodec/packet.h | 15 +++++++++++++++
libavformat/mov.c | 15 +++++++++++++++
3 files changed, 31 insertions(+)
diff --git a/Changelog b/Changelog
index 6f648bff2b..d51e836301 100644
--- a/Changelog
+++ b/Changelog
@@ -10,6 +10,7 @@ version <next>:
- ADPCM IMA Ubisoft APM encoder
- Rayman 2 APM muxer
- AV1 encoding support SVT-AV1
+- AV_PKT_DATA_EDIT_LIST added to AVStream side_data
version 4.3:
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index 0a19a0eff3..1f0e3405ed 100644
--- a/libavcodec/packet.h
+++ b/libavcodec/packet.h
@@ -290,6 +290,21 @@ enum AVPacketSideDataType {
*/
AV_PKT_DATA_S12M_TIMECODE,
+ /**
+ * ISO media file edit list side data packet
+ * The structre is repeated for each entry in the edit list
+ * The number of entries can be calculated
+ * by dividing the total size by the entry size
+ * Each entry is 20 bytes and is laid out as follows:
+ * @code
+ * s64be segment duration
+ * s64be media time
+ * s16be media rate
+ * s16be media rate fraction
+ * @endcode
+ */
+ AV_PKT_DATA_EDIT_LIST,
+
/**
* The number of side data types.
* This is not part of the public API/ABI in the sense that it may
diff --git a/libavformat/mov.c b/libavformat/mov.c
index d16840f3df..fbfcdddf3f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c,
AVIOContext *pb, MOVAtom atom)
&& sc->time_scale == st->codecpar->sample_rate) {
st->need_parsing = AVSTREAM_PARSE_FULL;
}
+
+ if (sc->elst_data) {
+ int i;
+ uint8_t *elst_data;
+ elst_data = av_stream_new_side_data(st,
AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20);
+ if (elst_data) {
+ for (i = 0; i < sc->elst_count; i++) {
+ uint32_t media_rate = (uint32_t)sc->elst_data[i].rate
* 65536.0;
+ AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration);
+ AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time);
+ AV_WB32((elst_data+(i*20)+16), media_rate);
+ }
+ }
+ }
+
/* Do not need those anymore. */
av_freep(&sc->chunk_offsets);
av_freep(&sc->sample_sizes);
On Wed, Jul 29, 2020 at 4:02 PM Matthew Szatmary <matt@szatmary.org> wrote: > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > --- > Changelog | 1 + > libavcodec/packet.h | 15 +++++++++++++++ > libavformat/mov.c | 15 +++++++++++++++ > 3 files changed, 31 insertions(+) > > diff --git a/Changelog b/Changelog > index 6f648bff2b..d51e836301 100644 > --- a/Changelog > +++ b/Changelog > @@ -10,6 +10,7 @@ version <next>: > - ADPCM IMA Ubisoft APM encoder > - Rayman 2 APM muxer > - AV1 encoding support SVT-AV1 > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > version 4.3: > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index 0a19a0eff3..1f0e3405ed 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -290,6 +290,21 @@ enum AVPacketSideDataType { > */ > AV_PKT_DATA_S12M_TIMECODE, > > + /** > + * ISO media file edit list side data packet > + * The structre is repeated for each entry in the edit list > + * The number of entries can be calculated > + * by dividing the total size by the entry size > + * Each entry is 20 bytes and is laid out as follows: > + * @code > + * s64be segment duration > + * s64be media time > + * s16be media rate > + * s16be media rate fraction > + * @endcode > + */ > + AV_PKT_DATA_EDIT_LIST, > This seems to be really MP4 centric. Did you check if such patch sent out for RFC in 2018 could do it? https://patchwork.ffmpeg.org/project/ffmpeg/patch/1522179841-34881-2-git-send-email-derek.buitenhuis@gmail.com/ > + > /** > * The number of side data types. > * This is not part of the public API/ABI in the sense that it may > diff --git a/libavformat/mov.c b/libavformat/mov.c > index d16840f3df..fbfcdddf3f 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, > AVIOContext *pb, MOVAtom atom) > && sc->time_scale == st->codecpar->sample_rate) { > st->need_parsing = AVSTREAM_PARSE_FULL; > } > + > + if (sc->elst_data) { > + int i; > + uint8_t *elst_data; > + elst_data = av_stream_new_side_data(st, > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > + if (elst_data) { > + for (i = 0; i < sc->elst_count; i++) { > + uint32_t media_rate = (uint32_t)sc->elst_data[i].rate > * 65536.0; > + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); > + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); > + AV_WB32((elst_data+(i*20)+16), media_rate); > Don't you need to know the global timescale and the stream timescale to make sense of those values? > + } > + } > + } > + > /* Do not need those anymore. */ > av_freep(&sc->chunk_offsets); > av_freep(&sc->sample_sizes); > -- > 2.27.0 > > On Wed, Jul 29, 2020 at 3:56 PM Matthew Szatmary <matt@szatmary.org> > wrote: > > > > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > > <andreas.rheinhardt@gmail.com> wrote: > > > > > > Matthew Szatmary: > > > > Create and populate AVStream side data packet with contents of > ISOBMFF > > > > edit list entries > > > > > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > > > --- > > > > Changelog | 1 + > > > > libavcodec/packet.h | 14 ++++++++++++++ > > > > libavformat/mov.c | 17 ++++++++++++++++- > > > > 3 files changed, 31 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Changelog b/Changelog > > > > index c37ffa82e1..2d719dd3b1 100644 > > > > --- a/Changelog > > > > +++ b/Changelog > > > > @@ -9,6 +9,7 @@ version <next>: > > > > - VDPAU accelerated HEVC 10/12bit decoding > > > > - ADPCM IMA Ubisoft APM encoder > > > > - Rayman 2 APM muxer > > > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > > > > > > > version 4.3: > > > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > > > index 0a19a0eff3..5faa594cf5 100644 > > > > --- a/libavcodec/packet.h > > > > +++ b/libavcodec/packet.h > > > > @@ -290,6 +290,20 @@ enum AVPacketSideDataType { > > > > */ > > > > AV_PKT_DATA_S12M_TIMECODE, > > > > > > > > + /** > > > > + * ISO media file edit list side data packet > > > > + * The structure is repeated for each entry in the edit list > > > > + * The number of entries can be calculated > > > > + * by dividing the packet size by the entry size > > > > + * Each entry is 20 bytes and is laid out as follows: > > > > + * @code > > > > + * s64le duration > > > > + * s64le time > > > > + * float32le rate > > Good point, I will make that change. > > > > > > > > You are obviously copying the MOVElst structure; yet the rate is a > 16.16 > > > fixed point number in the file and not a float, so one should probably > > > use this. > > > > > > > + * @endcode > > > > + */ > > > > + AV_PKT_DATA_EDIT_LIST, > > > > + > > > > /** > > > > * The number of side data types. > > > > * This is not part of the public API/ABI in the sense that it > may > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > index d16840f3df..bb2c940e80 100644 > > > > --- a/libavformat/mov.c > > > > +++ b/libavformat/mov.c > > > > @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, > > > > AVIOContext *pb, MOVAtom atom) > > > > av_freep(&sc->keyframes); > > > > av_freep(&sc->stts_data); > > > > av_freep(&sc->stps_data); > > > > - av_freep(&sc->elst_data); > > > > > > This is still needed, namely if an error happens before you can attach > > > the stream side-data. E.g. if an invalid edit list is found and > > > standards compliance is set to strict. Or if av_stream_new_side_data() > > > fails. > > > > av_freep(&sc->elst_data); is also called in mov_read_close, So it > > would only leak if the API was used incorrectly. But that said, I > > think I can move the logic to mov_read_trak, and make the whole point > > moot. I was just trying to keep it near the other side_data calls. > > > > > > > > > av_freep(&sc->rap_group); > > > > > > > > return 0; > > > > @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) > > > > AVStream *st = s->streams[i]; > > > > MOVStreamContext *sc = st->priv_data; > > > > > > > > + if (sc->elst_data) { > > > > + uint8_t *elst_data; > > > > + elst_data = av_stream_new_side_data(st, > > > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > > > > > I wonder whether it would be advantageouos to use > > > av_stream_add_side_data() here. > > > > av_stream_new_side_data is just a wrapper for av_malloc + > > av_stream_add_side_data > > > > > > + > > > > + if (!elst_data) > > > > + goto fail; > > > > + > > > > + for (j = 0; j < sc->elst_count; j++) { > > > > + AV_WB64((elst_data+(j*20)), > sc->elst_data[j].duration); > > > > + AV_WB64((elst_data+(j*20)+8), > sc->elst_data[j].time); > > > > > > "WB" stands for "Write Big-endian", yet your documentation says that it > > > is supposed to be little-endian. > > > > thanks, new patch included > > > > > > > > > > > + AV_WB32((elst_data+(j*20)+16), > sc->elst_data[j].rate); > > > > + } > > > > + > > > > + av_freep(&sc->elst_data); > > > > + } > > > > + > > > > switch (st->codecpar->codec_type) { > > > > case AVMEDIA_TYPE_AUDIO: > > > > err = ff_replaygain_export(st, s->metadata); > > > > > > > > > > _______________________________________________ > > > 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". > > > > > > Create and populate AVStream side data packet with contents of ISOBMFF > > edit list entries > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > --- > > Changelog | 1 + > > libavcodec/packet.h | 15 +++++++++++++++ > > libavformat/mov.c | 15 +++++++++++++++ > > 3 files changed, 31 insertions(+) > > > > diff --git a/Changelog b/Changelog > > index 6f648bff2b..d51e836301 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -10,6 +10,7 @@ version <next>: > > - ADPCM IMA Ubisoft APM encoder > > - Rayman 2 APM muxer > > - AV1 encoding support SVT-AV1 > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > version 4.3: > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > index 0a19a0eff3..1f0e3405ed 100644 > > --- a/libavcodec/packet.h > > +++ b/libavcodec/packet.h > > @@ -290,6 +290,21 @@ enum AVPacketSideDataType { > > */ > > AV_PKT_DATA_S12M_TIMECODE, > > > > + /** > > + * ISO media file edit list side data packet > > + * The structure is repeated for each entry in the edit list > > + * The number of entries can be calculated > > + * by dividing the packet size by the entry size > > + * Each entry is 20 bytes and is laid out as follows: > > + * @code > > + * s64be segment duration > > + * s64be media time > > + * s16be media rate > > + * s16be media rate fraction > > + * @endcode > > + */ > > + AV_PKT_DATA_EDIT_LIST, > > + > > /** > > * The number of side data types. > > * This is not part of the public API/ABI in the sense that it may > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index d16840f3df..fbfcdddf3f 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, > > AVIOContext *pb, MOVAtom atom) > > && sc->time_scale == st->codecpar->sample_rate) { > > st->need_parsing = AVSTREAM_PARSE_FULL; > > } > > + > > + if (sc->elst_data) { > > + int i; > > + uint8_t *elst_data; > > + elst_data = av_stream_new_side_data(st, > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > + if (elst_data) { > > + for (i = 0; i < sc->elst_count; i++) { > > + uint32_t media_rate = > > (uint32_t)(sc->elst_data[i].rate * 65536.0); > > + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); > > + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); > > + AV_WB32((elst_data+(i*20)+16), media_rate); > > + } > > + } > > + } > > + > > /* Do not need those anymore. */ > > av_freep(&sc->chunk_offsets); > > av_freep(&sc->sample_sizes); > > -- > > 2.27.0 > _______________________________________________ > 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".
Matthew Szatmary: > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > <andreas.rheinhardt@gmail.com> wrote: >> >> Matthew Szatmary: >>> Create and populate AVStream side data packet with contents of ISOBMFF >>> edit list entries >>> >>> Signed-off-by: Matthew Szatmary <matt@szatmary.org> >>> --- >>> Changelog | 1 + >>> libavcodec/packet.h | 14 ++++++++++++++ >>> libavformat/mov.c | 17 ++++++++++++++++- >>> 3 files changed, 31 insertions(+), 1 deletion(-) >>> >>> diff --git a/Changelog b/Changelog >>> index c37ffa82e1..2d719dd3b1 100644 >>> --- a/Changelog >>> +++ b/Changelog >>> @@ -9,6 +9,7 @@ version <next>: >>> - VDPAU accelerated HEVC 10/12bit decoding >>> - ADPCM IMA Ubisoft APM encoder >>> - Rayman 2 APM muxer >>> +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data >>> >>> >>> version 4.3: >>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h >>> index 0a19a0eff3..5faa594cf5 100644 >>> --- a/libavcodec/packet.h >>> +++ b/libavcodec/packet.h >>> @@ -290,6 +290,20 @@ enum AVPacketSideDataType { >>> */ >>> AV_PKT_DATA_S12M_TIMECODE, >>> >>> + /** >>> + * ISO media file edit list side data packet >>> + * The structure is repeated for each entry in the edit list >>> + * The number of entries can be calculated >>> + * by dividing the packet size by the entry size >>> + * Each entry is 20 bytes and is laid out as follows: >>> + * @code >>> + * s64le duration >>> + * s64le time >>> + * float32le rate > Good point, I will make that change. > >> >> You are obviously copying the MOVElst structure; yet the rate is a 16.16 >> fixed point number in the file and not a float, so one should probably >> use this. >> >>> + * @endcode >>> + */ >>> + AV_PKT_DATA_EDIT_LIST, >>> + >>> /** >>> * The number of side data types. >>> * This is not part of the public API/ABI in the sense that it may >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index d16840f3df..bb2c940e80 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, >>> AVIOContext *pb, MOVAtom atom) >>> av_freep(&sc->keyframes); >>> av_freep(&sc->stts_data); >>> av_freep(&sc->stps_data); >>> - av_freep(&sc->elst_data); >> >> This is still needed, namely if an error happens before you can attach >> the stream side-data. E.g. if an invalid edit list is found and >> standards compliance is set to strict. Or if av_stream_new_side_data() >> fails. > > av_freep(&sc->elst_data); is also called in mov_read_close, So it > would only leak if the API was used incorrectly. But that said, I > think I can move the logic to mov_read_trak, and make the whole point > moot. I was just trying to keep it near the other side_data calls. > Sorry, I thought you were removing the av_freep() from mov_read_close(); I didn't realize that it would also be freed in mov_read_trak(). >> >>> av_freep(&sc->rap_group); >>> >>> return 0; >>> @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) >>> AVStream *st = s->streams[i]; >>> MOVStreamContext *sc = st->priv_data; >>> >>> + if (sc->elst_data) { >>> + uint8_t *elst_data; >>> + elst_data = av_stream_new_side_data(st, >>> AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); >> >> I wonder whether it would be advantageouos to use >> av_stream_add_side_data() here. > > av_stream_new_side_data is just a wrapper for av_malloc + > av_stream_add_side_data Yes, and as such I hoped that it could be used to avoid the allocation; but it is impossible: The MOVElst struct will have padding at the end so that its size is 24 (ordinarily; compilers are free to insert even more padding), so that it can't be reused anyway. > >>> + >>> + if (!elst_data) >>> + goto fail; >>> + >>> + for (j = 0; j < sc->elst_count; j++) { >>> + AV_WB64((elst_data+(j*20)), sc->elst_data[j].duration); >>> + AV_WB64((elst_data+(j*20)+8), sc->elst_data[j].time); >> >> "WB" stands for "Write Big-endian", yet your documentation says that it >> is supposed to be little-endian. > > thanks, new patch included > > >> >>> + AV_WB32((elst_data+(j*20)+16), sc->elst_data[j].rate); >>> + } >>> + >>> + av_freep(&sc->elst_data); >>> + } >>> + >>> switch (st->codecpar->codec_type) { >>> case AVMEDIA_TYPE_AUDIO: >>> err = ff_replaygain_export(st, s->metadata); >>> >> >> _______________________________________________ >> 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". > > > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > --- > Changelog | 1 + > libavcodec/packet.h | 15 +++++++++++++++ > libavformat/mov.c | 15 +++++++++++++++ > 3 files changed, 31 insertions(+) > > diff --git a/Changelog b/Changelog > index 6f648bff2b..d51e836301 100644 > --- a/Changelog > +++ b/Changelog > @@ -10,6 +10,7 @@ version <next>: > - ADPCM IMA Ubisoft APM encoder > - Rayman 2 APM muxer > - AV1 encoding support SVT-AV1 > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > version 4.3: > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > index 0a19a0eff3..1f0e3405ed 100644 > --- a/libavcodec/packet.h > +++ b/libavcodec/packet.h > @@ -290,6 +290,21 @@ enum AVPacketSideDataType { > */ > AV_PKT_DATA_S12M_TIMECODE, > > + /** > + * ISO media file edit list side data packet > + * The structure is repeated for each entry in the edit list > + * The number of entries can be calculated > + * by dividing the packet size by the entry size > + * Each entry is 20 bytes and is laid out as follows: > + * @code > + * s64be segment duration > + * s64be media time > + * s16be media rate > + * s16be media rate fraction > + * @endcode > + */ > + AV_PKT_DATA_EDIT_LIST, > + > /** > * The number of side data types. > * This is not part of the public API/ABI in the sense that it may > diff --git a/libavformat/mov.c b/libavformat/mov.c > index d16840f3df..fbfcdddf3f 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, > AVIOContext *pb, MOVAtom atom) > && sc->time_scale == st->codecpar->sample_rate) { > st->need_parsing = AVSTREAM_PARSE_FULL; > } > + > + if (sc->elst_data) { > + int i; > + uint8_t *elst_data; > + elst_data = av_stream_new_side_data(st, > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > + if (elst_data) { > + for (i = 0; i < sc->elst_count; i++) { > + uint32_t media_rate = > (uint32_t)(sc->elst_data[i].rate * 65536.0); > + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); > + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); > + AV_WB32((elst_data+(i*20)+16), media_rate); > + } > + } > + } > + > /* Do not need those anymore. */ > av_freep(&sc->chunk_offsets); > av_freep(&sc->sample_sizes); >
On Wed, Jul 29, 2020 at 4:17 PM Thierry Foucu <tfoucu@gmail.com> wrote: > > On Wed, Jul 29, 2020 at 4:02 PM Matthew Szatmary <matt@szatmary.org> wrote: > > > Create and populate AVStream side data packet with contents of ISOBMFF > > edit list entries > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > --- > > Changelog | 1 + > > libavcodec/packet.h | 15 +++++++++++++++ > > libavformat/mov.c | 15 +++++++++++++++ > > 3 files changed, 31 insertions(+) > > > > diff --git a/Changelog b/Changelog > > index 6f648bff2b..d51e836301 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -10,6 +10,7 @@ version <next>: > > - ADPCM IMA Ubisoft APM encoder > > - Rayman 2 APM muxer > > - AV1 encoding support SVT-AV1 > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > version 4.3: > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > index 0a19a0eff3..1f0e3405ed 100644 > > --- a/libavcodec/packet.h > > +++ b/libavcodec/packet.h > > @@ -290,6 +290,21 @@ enum AVPacketSideDataType { > > */ > > AV_PKT_DATA_S12M_TIMECODE, > > > > + /** > > + * ISO media file edit list side data packet > > + * The structre is repeated for each entry in the edit list > > + * The number of entries can be calculated > > + * by dividing the total size by the entry size > > + * Each entry is 20 bytes and is laid out as follows: > > + * @code > > + * s64be segment duration > > + * s64be media time > > + * s16be media rate > > + * s16be media rate fraction > > + * @endcode > > + */ > > + AV_PKT_DATA_EDIT_LIST, > > > > This seems to be really MP4 centric. Did you check if such patch sent out > for RFC in 2018 could do it? > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/1522179841-34881-2-git-send-email-derek.buitenhuis@gmail.com/ > I was unaware of this patch, And it does not seem to included into master branch > > > + > > /** > > * The number of side data types. > > * This is not part of the public API/ABI in the sense that it may > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index d16840f3df..fbfcdddf3f 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, > > AVIOContext *pb, MOVAtom atom) > > && sc->time_scale == st->codecpar->sample_rate) { > > st->need_parsing = AVSTREAM_PARSE_FULL; > > } > > + > > + if (sc->elst_data) { > > + int i; > > + uint8_t *elst_data; > > + elst_data = av_stream_new_side_data(st, > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > + if (elst_data) { > > + for (i = 0; i < sc->elst_count; i++) { > > + uint32_t media_rate = (uint32_t)sc->elst_data[i].rate > > * 65536.0; > > + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); > > + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); > > + AV_WB32((elst_data+(i*20)+16), media_rate); > > > > Don't you need to know the global timescale and the stream timescale to > make sense of those values? > Excellent point! I did not need to for my specific use case, But that oversight does make this useless for many use cases. I am resubmit a patch converting those values to AV_TIME_BASE > > > + } > > + } > > + } > > + > > /* Do not need those anymore. */ > > av_freep(&sc->chunk_offsets); > > av_freep(&sc->sample_sizes); > > -- > > 2.27.0 > > > > On Wed, Jul 29, 2020 at 3:56 PM Matthew Szatmary <matt@szatmary.org> > > wrote: > > > > > > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > > > <andreas.rheinhardt@gmail.com> wrote: > > > > > > > > Matthew Szatmary: > > > > > Create and populate AVStream side data packet with contents of > > ISOBMFF > > > > > edit list entries > > > > > > > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > > > > --- > > > > > Changelog | 1 + > > > > > libavcodec/packet.h | 14 ++++++++++++++ > > > > > libavformat/mov.c | 17 ++++++++++++++++- > > > > > 3 files changed, 31 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/Changelog b/Changelog > > > > > index c37ffa82e1..2d719dd3b1 100644 > > > > > --- a/Changelog > > > > > +++ b/Changelog > > > > > @@ -9,6 +9,7 @@ version <next>: > > > > > - VDPAU accelerated HEVC 10/12bit decoding > > > > > - ADPCM IMA Ubisoft APM encoder > > > > > - Rayman 2 APM muxer > > > > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > > > > > > > > > > version 4.3: > > > > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > > > > index 0a19a0eff3..5faa594cf5 100644 > > > > > --- a/libavcodec/packet.h > > > > > +++ b/libavcodec/packet.h > > > > > @@ -290,6 +290,20 @@ enum AVPacketSideDataType { > > > > > */ > > > > > AV_PKT_DATA_S12M_TIMECODE, > > > > > > > > > > + /** > > > > > + * ISO media file edit list side data packet > > > > > + * The structure is repeated for each entry in the edit list > > > > > + * The number of entries can be calculated > > > > > + * by dividing the packet size by the entry size > > > > > + * Each entry is 20 bytes and is laid out as follows: > > > > > + * @code > > > > > + * s64le duration > > > > > + * s64le time > > > > > + * float32le rate > > > Good point, I will make that change. > > > > > > > > > > > You are obviously copying the MOVElst structure; yet the rate is a > > 16.16 > > > > fixed point number in the file and not a float, so one should probably > > > > use this. > > > > > > > > > + * @endcode > > > > > + */ > > > > > + AV_PKT_DATA_EDIT_LIST, > > > > > + > > > > > /** > > > > > * The number of side data types. > > > > > * This is not part of the public API/ABI in the sense that it > > may > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > > index d16840f3df..bb2c940e80 100644 > > > > > --- a/libavformat/mov.c > > > > > +++ b/libavformat/mov.c > > > > > @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, > > > > > AVIOContext *pb, MOVAtom atom) > > > > > av_freep(&sc->keyframes); > > > > > av_freep(&sc->stts_data); > > > > > av_freep(&sc->stps_data); > > > > > - av_freep(&sc->elst_data); > > > > > > > > This is still needed, namely if an error happens before you can attach > > > > the stream side-data. E.g. if an invalid edit list is found and > > > > standards compliance is set to strict. Or if av_stream_new_side_data() > > > > fails. > > > > > > av_freep(&sc->elst_data); is also called in mov_read_close, So it > > > would only leak if the API was used incorrectly. But that said, I > > > think I can move the logic to mov_read_trak, and make the whole point > > > moot. I was just trying to keep it near the other side_data calls. > > > > > > > > > > > > av_freep(&sc->rap_group); > > > > > > > > > > return 0; > > > > > @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) > > > > > AVStream *st = s->streams[i]; > > > > > MOVStreamContext *sc = st->priv_data; > > > > > > > > > > + if (sc->elst_data) { > > > > > + uint8_t *elst_data; > > > > > + elst_data = av_stream_new_side_data(st, > > > > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > > > > > > > I wonder whether it would be advantageouos to use > > > > av_stream_add_side_data() here. > > > > > > av_stream_new_side_data is just a wrapper for av_malloc + > > > av_stream_add_side_data > > > > > > > > + > > > > > + if (!elst_data) > > > > > + goto fail; > > > > > + > > > > > + for (j = 0; j < sc->elst_count; j++) { > > > > > + AV_WB64((elst_data+(j*20)), > > sc->elst_data[j].duration); > > > > > + AV_WB64((elst_data+(j*20)+8), > > sc->elst_data[j].time); > > > > > > > > "WB" stands for "Write Big-endian", yet your documentation says that it > > > > is supposed to be little-endian. > > > > > > thanks, new patch included > > > > > > > > > > > > > > > + AV_WB32((elst_data+(j*20)+16), > > sc->elst_data[j].rate); > > > > > + } > > > > > + > > > > > + av_freep(&sc->elst_data); > > > > > + } > > > > > + > > > > > switch (st->codecpar->codec_type) { > > > > > case AVMEDIA_TYPE_AUDIO: > > > > > err = ff_replaygain_export(st, s->metadata); > > > > > > > > > > > > > _______________________________________________ > > > > 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". > > > > > > > > > Create and populate AVStream side data packet with contents of ISOBMFF > > > edit list entries > > > > > > Signed-off-by: Matthew Szatmary <matt@szatmary.org> > > > --- > > > Changelog | 1 + > > > libavcodec/packet.h | 15 +++++++++++++++ > > > libavformat/mov.c | 15 +++++++++++++++ > > > 3 files changed, 31 insertions(+) > > > > > > diff --git a/Changelog b/Changelog > > > index 6f648bff2b..d51e836301 100644 > > > --- a/Changelog > > > +++ b/Changelog > > > @@ -10,6 +10,7 @@ version <next>: > > > - ADPCM IMA Ubisoft APM encoder > > > - Rayman 2 APM muxer > > > - AV1 encoding support SVT-AV1 > > > +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data > > > > > > > > > version 4.3: > > > diff --git a/libavcodec/packet.h b/libavcodec/packet.h > > > index 0a19a0eff3..1f0e3405ed 100644 > > > --- a/libavcodec/packet.h > > > +++ b/libavcodec/packet.h > > > @@ -290,6 +290,21 @@ enum AVPacketSideDataType { > > > */ > > > AV_PKT_DATA_S12M_TIMECODE, > > > > > > + /** > > > + * ISO media file edit list side data packet > > > + * The structure is repeated for each entry in the edit list > > > + * The number of entries can be calculated > > > + * by dividing the packet size by the entry size > > > + * Each entry is 20 bytes and is laid out as follows: > > > + * @code > > > + * s64be segment duration > > > + * s64be media time > > > + * s16be media rate > > > + * s16be media rate fraction > > > + * @endcode > > > + */ > > > + AV_PKT_DATA_EDIT_LIST, > > > + > > > /** > > > * The number of side data types. > > > * This is not part of the public API/ABI in the sense that it may > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > index d16840f3df..fbfcdddf3f 100644 > > > --- a/libavformat/mov.c > > > +++ b/libavformat/mov.c > > > @@ -4311,6 +4311,21 @@ static int mov_read_trak(MOVContext *c, > > > AVIOContext *pb, MOVAtom atom) > > > && sc->time_scale == st->codecpar->sample_rate) { > > > st->need_parsing = AVSTREAM_PARSE_FULL; > > > } > > > + > > > + if (sc->elst_data) { > > > + int i; > > > + uint8_t *elst_data; > > > + elst_data = av_stream_new_side_data(st, > > > AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); > > > + if (elst_data) { > > > + for (i = 0; i < sc->elst_count; i++) { > > > + uint32_t media_rate = > > > (uint32_t)(sc->elst_data[i].rate * 65536.0); > > > + AV_WB64((elst_data+(i*20)), sc->elst_data[i].duration); > > > + AV_WB64((elst_data+(i*20)+8), sc->elst_data[i].time); > > > + AV_WB32((elst_data+(i*20)+16), media_rate); > > > + } > > > + } > > > + } > > > + > > > /* Do not need those anymore. */ > > > av_freep(&sc->chunk_offsets); > > > av_freep(&sc->sample_sizes); > > > -- > > > 2.27.0 > > _______________________________________________ > > 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". > > > > -- > > Thierry Foucu > _______________________________________________ > 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".
From 91ae719dd69cba5153e4ce1aad2eb3ce819668b1 Mon Sep 17 00:00:00 2001 From: Matthew Szatmary <matt@szatmary.org> Date: Thu, 23 Jul 2020 16:36:49 -0700 Subject: [PATCH] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries Signed-off-by: Matthew Szatmary <matt@szatmary.org> --- Changelog | 1 + libavcodec/packet.h | 15 +++++++++++++++ libavformat/mov.c | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/Changelog b/Changelog index 6f648bff2b..d51e836301 100644 --- a/Changelog +++ b/Changelog @@ -10,6 +10,7 @@ version <next>: - ADPCM IMA Ubisoft APM encoder - Rayman 2 APM muxer - AV1 encoding support SVT-AV1 +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data version 4.3: diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 0a19a0eff3..f4a00ccf1e 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -290,6 +290,21 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_S12M_TIMECODE, + /** + * ISO media file edit list side data packet + * The structre is repeated for each entry in the edit list + * The number of entries can be calculated + * by dividing the total size by the entry size + * Each entry is 20 bytes and is laid out as follows: + * @code + * s64be segment duration in AV_TIME_BASE + * s64be media time in AV_TIME_BASE + * s16be media rate + * s16be media rate fraction + * @endcode + */ + AV_PKT_DATA_EDIT_LIST, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/mov.c b/libavformat/mov.c index d16840f3df..0d29588831 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4311,6 +4311,25 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) && sc->time_scale == st->codecpar->sample_rate) { st->need_parsing = AVSTREAM_PARSE_FULL; } + + if (sc->elst_data) { + int i; + uint8_t *elst_data; + elst_data = av_stream_new_side_data(st, AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); + if (elst_data) { + for (i = 0; i < sc->elst_count; i++) { + uint32_t media_rate; + int64_t duration, time; + media_rate = (uint32_t)(sc->elst_data[i].rate * 65536.0); + duration = av_rescale(sc->elst_data[i].duration, c->time_scale, AV_TIME_BASE); + time = av_rescale(sc->elst_data[i].time, c->time_scale, AV_TIME_BASE); + AV_WB64((elst_data+(i*20)), duration); + AV_WB64((elst_data+(i*20)+8), time); + AV_WB32((elst_data+(i*20)+16), media_rate); + } + } + } + /* Do not need those anymore. */ av_freep(&sc->chunk_offsets); av_freep(&sc->sample_sizes);
On 30/07/2020 16:55, Matthew Szatmary wrote: >> This seems to be really MP4 centric. Did you check if such patch sent out >> for RFC in 2018 could do it? >> >> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1522179841-34881-2-git-send-email-derek.buitenhuis@gmail.com/ >> > I was unaware of this patch, And it does not seem to included into master branch Yeah, that kind of just... dropped off my radar and a I never sent a v2. I don't really really have a strong opinion on whether to pick that 2 year old patch up again, or continue with this current patch from Matthew. I don't really think it matters, as long as anything pushed isn't too rigid to extend to use with ordered chapters or other cosmic horrors involving MXF. - Derek
diff --git a/Changelog b/Changelog index c37ffa82e1..2d719dd3b1 100644 --- a/Changelog +++ b/Changelog @@ -9,6 +9,7 @@ version <next>: - VDPAU accelerated HEVC 10/12bit decoding - ADPCM IMA Ubisoft APM encoder - Rayman 2 APM muxer +- AV_PKT_DATA_EDIT_LIST added to AVStream side_data version 4.3: diff --git a/libavcodec/packet.h b/libavcodec/packet.h index 0a19a0eff3..5faa594cf5 100644 --- a/libavcodec/packet.h +++ b/libavcodec/packet.h @@ -290,6 +290,20 @@ enum AVPacketSideDataType { */ AV_PKT_DATA_S12M_TIMECODE, + /** + * ISO media file edit list side data packet + * The structure is repeated for each entry in the edit list + * The number of entries can be calculated + * by dividing the packet size by the entry size + * Each entry is 20 bytes and is laid out as follows: + * @code + * s64le duration + * s64le time + * float32le rate + * @endcode + */ + AV_PKT_DATA_EDIT_LIST, + /** * The number of side data types. * This is not part of the public API/ABI in the sense that it may diff --git a/libavformat/mov.c b/libavformat/mov.c index d16840f3df..bb2c940e80 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4317,7 +4317,6 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom) av_freep(&sc->keyframes); av_freep(&sc->stts_data); av_freep(&sc->stps_data); - av_freep(&sc->elst_data); av_freep(&sc->rap_group);
Create and populate AVStream side data packet with contents of ISOBMFF edit list entries Signed-off-by: Matthew Szatmary <matt@szatmary.org> --- Changelog | 1 + libavcodec/packet.h | 14 ++++++++++++++ libavformat/mov.c | 17 ++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) return 0; @@ -7662,6 +7661,22 @@ static int mov_read_header(AVFormatContext *s) AVStream *st = s->streams[i]; MOVStreamContext *sc = st->priv_data; + if (sc->elst_data) { + uint8_t *elst_data; + elst_data = av_stream_new_side_data(st, AV_PKT_DATA_EDIT_LIST, sc->elst_count * 20); + + if (!elst_data) + goto fail; + + for (j = 0; j < sc->elst_count; j++) { + AV_WB64((elst_data+(j*20)), sc->elst_data[j].duration); + AV_WB64((elst_data+(j*20)+8), sc->elst_data[j].time); + AV_WB32((elst_data+(j*20)+16), sc->elst_data[j].rate); + } + + av_freep(&sc->elst_data); + } + switch (st->codecpar->codec_type) { case AVMEDIA_TYPE_AUDIO: err = ff_replaygain_export(st, s->metadata);