Matthew Szatmary:
> On Wed, Jul 29, 2020 at 12:22 PM Andreas Rheinhardt
> <andreas.rheinha...@gmail.com> wrote:
>>
>> Matthew Szatmary:
>>> Create and populate AVStream side data packet with contents of ISOBMFF
>>> edit list entries
>>>
>>> Signed-off-by: Matthew Szatmary <m...@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-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 <m...@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);
>
_______________________________________________
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".
