Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
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.buitenh...@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 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
From 91ae719dd69cba5153e4ce1aad2eb3ce819668b1 Mon Sep 17 00:00:00 2001 From: Matthew Szatmary 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 --- 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 : - 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(>chunk_offsets); av_freep(>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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
On Wed, Jul 29, 2020 at 4:17 PM Thierry Foucu wrote: > > On Wed, Jul 29, 2020 at 4:02 PM Matthew Szatmary wrote: > > > Create and populate AVStream side data packet with contents of ISOBMFF > > edit list entries > > > > Signed-off-by: Matthew Szatmary > > --- > > 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 : > > - 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.buitenh...@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(>chunk_offsets); > > av_freep(>sample_sizes); > > -- > > 2.27.0 > > > > On Wed, Jul 29, 2020 at 3:56 PM Matthew Szatmary > > wrote: > > > > > > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > > > wrote: > > > > > > > > Matthew Szatmary: > > > > > Create and populate AVStream side data packet with contents of > > ISOBMFF > > > > > edit list entries > > > > > > > > > > Signed-off-by: Matthew Szatmary > > > > > --- > > > > > 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 : > > > > > - 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
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
Matthew Szatmary: > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > wrote: >> >> Matthew Szatmary: >>> Create and populate AVStream side data packet with contents of ISOBMFF >>> edit list entries >>> >>> Signed-off-by: Matthew Szatmary >>> --- >>> 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 : >>> - 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(>keyframes); >>> av_freep(>stts_data); >>> av_freep(>stps_data); >>> -av_freep(>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(>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(>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(>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-requ...@ffmpeg.org with subject "unsubscribe". > > > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary > --- > Changelog | 1 + > libavcodec/packet.h | 15 +++ >
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
On Wed, Jul 29, 2020 at 4:02 PM Matthew Szatmary wrote: > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary > --- > 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 : > - 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.buitenh...@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(>chunk_offsets); > av_freep(>sample_sizes); > -- > 2.27.0 > > On Wed, Jul 29, 2020 at 3:56 PM Matthew Szatmary > wrote: > > > > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > > wrote: > > > > > > Matthew Szatmary: > > > > Create and populate AVStream side data packet with contents of > ISOBMFF > > > > edit list entries > > > > > > > > Signed-off-by: Matthew Szatmary > > > > --- > > > > 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 : > > > > - 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
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
Create and populate AVStream side data packet with contents of ISOBMFF edit list entries Signed-off-by: Matthew Szatmary --- 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 : - 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(>chunk_offsets); av_freep(>sample_sizes); -- 2.27.0 On Wed, Jul 29, 2020 at 3:56 PM Matthew Szatmary wrote: > > On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt > wrote: > > > > Matthew Szatmary: > > > Create and populate AVStream side data packet with contents of ISOBMFF > > > edit list entries > > > > > > Signed-off-by: Matthew Szatmary > > > --- > > > 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 : > > > - 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(>keyframes); > > > av_freep(>stts_data); > > > av_freep(>stps_data); > > > -av_freep(>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
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt wrote: > > Matthew Szatmary: > > Create and populate AVStream side data packet with contents of ISOBMFF > > edit list entries > > > > Signed-off-by: Matthew Szatmary > > --- > > 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 : > > - 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(>keyframes); > > av_freep(>stts_data); > > av_freep(>stps_data); > > -av_freep(>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(>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(>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(>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-requ...@ffmpeg.org with subject "unsubscribe". Create and populate AVStream side data packet with contents of ISOBMFF edit list entries Signed-off-by: Matthew Szatmary --- 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 : - 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 @@
Re: [FFmpeg-devel] Create and populate AVStream side data packet with contents of ISOBMFF edit list entries
Matthew Szatmary: > Create and populate AVStream side data packet with contents of ISOBMFF > edit list entries > > Signed-off-by: Matthew Szatmary > --- > 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 : > - 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(>keyframes); > av_freep(>stts_data); > av_freep(>stps_data); > -av_freep(>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(>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(>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-requ...@ffmpeg.org with subject "unsubscribe".