| Message ID | AS8P250MB0744AE847962511BE05158218FCAA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/matroskadec: Output palette as stream side data | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
On Thu, Oct 05, 2023 at 02:36:52PM +0200, Andreas Rheinhardt wrote: > This is simpler implementation-wise (it avoids an almost-always-false > check in read_packet and decreases sizeof(MatroskaTrack) by about 2/3) > and makes the side-data available directly after read_header. > > It also fixes the Matroska analog of ticket #10602: If a Matroska track > has a palette, said palette will be attached as side-data to the > first packet of said track and most likely, this is one read during > avformat_find_stream_info(). Yet if this packet is discarded because > of a seek performed immediately after avformat_find_stream_info(), > the information about the global palette will never reach the user. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > Ticket #10602 can unfortunately not be fixed in this way, > because avi allows to update the palette mid-stream, > so that the palette contained in extradata must not be > exported via global side data due to the semantics of the latter. > > libavformat/matroskadec.c | 39 ++++++++++++--------------------------- > 1 file changed, 12 insertions(+), 27 deletions(-) breaks: aletrek.mkv probably here: https://trac.ffmpeg.org/attachment/ticket/5071/aletrek.mkv thx [...]
Michael Niedermayer: > On Thu, Oct 05, 2023 at 02:36:52PM +0200, Andreas Rheinhardt wrote: >> This is simpler implementation-wise (it avoids an almost-always-false >> check in read_packet and decreases sizeof(MatroskaTrack) by about 2/3) >> and makes the side-data available directly after read_header. >> >> It also fixes the Matroska analog of ticket #10602: If a Matroska track >> has a palette, said palette will be attached as side-data to the >> first packet of said track and most likely, this is one read during >> avformat_find_stream_info(). Yet if this packet is discarded because >> of a seek performed immediately after avformat_find_stream_info(), >> the information about the global palette will never reach the user. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> Ticket #10602 can unfortunately not be fixed in this way, >> because avi allows to update the palette mid-stream, >> so that the palette contained in extradata must not be >> exported via global side data due to the semantics of the latter. >> >> libavformat/matroskadec.c | 39 ++++++++++++--------------------------- >> 1 file changed, 12 insertions(+), 27 deletions(-) > > breaks: > aletrek.mkv > > probably here: > https://trac.ffmpeg.org/attachment/ticket/5071/aletrek.mkv > This is actually expected due to the recent changes with respect to side data: Basically every piece of code currently only looks for palette side data in packets, not in stream side data and if the latter is not injected into packets, then it is available, but unused. This is the reason I opposed deprecating avformat_inject_global_side_data(); see my reply to James latest iteration of his patchset. - Andreas
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 941c0bcdc9..74f3fc7ae7 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -285,9 +285,6 @@ typedef struct MatroskaTrack { int needs_decoding; uint64_t max_block_additional_id; EbmlList block_addition_mappings; - - uint32_t palette[AVPALETTE_COUNT]; - int has_palette; } MatroskaTrack; typedef struct MatroskaAttachment { @@ -2852,7 +2849,8 @@ static int mka_parse_audio(MatroskaTrack *track, AVStream *st, } /* Performs the codec-specific part of parsing a video track. */ -static int mkv_parse_video_codec(MatroskaTrack *track, AVCodecParameters *par, +static int mkv_parse_video_codec(AVStream *st, MatroskaTrack *track, + AVCodecParameters *par, const MatroskaDemuxContext *matroska, int *extradata_offset) { @@ -2886,11 +2884,15 @@ static int mkv_parse_video_codec(MatroskaTrack *track, AVCodecParameters *par, if (track->codec_priv.size >= 86) { FFIOContext b; unsigned bit_depth = AV_RB16(track->codec_priv.data + 82); + uint32_t palette[256] = { 0 }; ffio_init_read_context(&b, track->codec_priv.data, track->codec_priv.size); - if (ff_get_qtpalette(codec_id, &b.pub, track->palette)) { + if (ff_get_qtpalette(codec_id, &b.pub, palette)) { + void *side_data = av_stream_new_side_data(st, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); + if (!side_data) + return AVERROR(ENOMEM); + memcpy(side_data, palette, AVPALETTE_SIZE); bit_depth &= 0x1F; - track->has_palette = 1; } par->bits_per_coded_sample = bit_depth; } @@ -2934,7 +2936,7 @@ static int mkv_parse_video(MatroskaTrack *track, AVStream *st, if (track->video.color_space.size == 4) par->codec_tag = AV_RL32(track->video.color_space.data); - ret = mkv_parse_video_codec(track, par, matroska, + ret = mkv_parse_video_codec(st, track, par, matroska, extradata_offset); if (ret < 0) return ret; @@ -3398,30 +3400,13 @@ static int matroska_read_header(AVFormatContext *s) /* * Put one packet in an application-supplied AVPacket struct. - * Returns 0 on success or -1 on failure. + * Returns 0 on success or < 0 (most likely AVERROR(EAGAIN) + * if no packet was available. */ static int matroska_deliver_packet(MatroskaDemuxContext *matroska, AVPacket *pkt) { - if (matroska->queue.head) { - MatroskaTrack *tracks = matroska->tracks.elem; - MatroskaTrack *track; - - avpriv_packet_list_get(&matroska->queue, pkt); - track = &tracks[pkt->stream_index]; - if (track->has_palette) { - uint8_t *pal = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE); - if (!pal) { - av_log(matroska->ctx, AV_LOG_ERROR, "Cannot append palette to packet\n"); - } else { - memcpy(pal, track->palette, AVPALETTE_SIZE); - } - track->has_palette = 0; - } - return 0; - } - - return -1; + return avpriv_packet_list_get(&matroska->queue, pkt); } /*
This is simpler implementation-wise (it avoids an almost-always-false check in read_packet and decreases sizeof(MatroskaTrack) by about 2/3) and makes the side-data available directly after read_header. It also fixes the Matroska analog of ticket #10602: If a Matroska track has a palette, said palette will be attached as side-data to the first packet of said track and most likely, this is one read during avformat_find_stream_info(). Yet if this packet is discarded because of a seek performed immediately after avformat_find_stream_info(), the information about the global palette will never reach the user. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- Ticket #10602 can unfortunately not be fixed in this way, because avi allows to update the palette mid-stream, so that the palette contained in extradata must not be exported via global side data due to the semantics of the latter. libavformat/matroskadec.c | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-)