| Message ID | 20170619234238.1364-2-hp@tmm.cx |
|---|---|
| State | Superseded |
| Headers | show |
On 6/20/17, Hein-Pieter van Braam <hp@tmm.cx> wrote: > Interplay MVE movies have a SEND_BUFFER operation. Only after this > command does the current decoding buffer get displayed. This is required > for the other frame formats. They are fixed-size and can't always encode > a full frame worth of pixeldata. > > This code prevents half-finished frames from being emitted. > > Signed-off-by: Hein-Pieter van Braam <hp@tmm.cx> > --- > libavcodec/interplayvideo.c | 13 ++++++++----- > libavformat/ipmovie.c | 16 +++++++++++----- > 2 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/interplayvideo.c b/libavcodec/interplayvideo.c > index df3314d..9fed8f6 100644 > --- a/libavcodec/interplayvideo.c > +++ b/libavcodec/interplayvideo.c > @@ -990,6 +990,7 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, > IpvideoContext *s = avctx->priv_data; > AVFrame *frame = data; > int ret; > + int send_buffer; > > if (av_packet_get_side_data(avpkt, AV_PKT_DATA_PARAM_CHANGE, NULL)) { > av_frame_unref(s->last_frame); > @@ -999,8 +1000,10 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, > if (buf_size < 2) > return AVERROR_INVALIDDATA; Checks only for 2 bytes. > > + send_buffer = AV_RL8(avpkt->data); > + > /* decoding map contains 4 bits of information per 8x8 block */ > - s->decoding_map_size = AV_RL16(avpkt->data); > + s->decoding_map_size = AV_RL16(avpkt->data + 1); Reads up to 3 bytes from input buffer. So above check needs to be updated. > > /* compressed buffer needs to be large enough to at least hold an > entire > * decoding map */ > @@ -1008,9 +1011,9 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, > return buf_size; > > > - s->decoding_map = buf + 2; > - bytestream2_init(&s->stream_ptr, buf + 2 + s->decoding_map_size, > - buf_size - s->decoding_map_size); > + s->decoding_map = buf + 3; > + bytestream2_init(&s->stream_ptr, buf + 3 + s->decoding_map_size, > + buf_size - s->decoding_map_size - 3); > > if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0) > return ret; > @@ -1028,7 +1031,7 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, > > ipvideo_decode_opcodes(s, frame); > > - *got_frame = 1; > + *got_frame = send_buffer; > > /* shuffle frames */ > av_frame_unref(s->second_last_frame); > diff --git a/libavformat/ipmovie.c b/libavformat/ipmovie.c > index 29eeaf6..0705d33 100644 > --- a/libavformat/ipmovie.c > +++ b/libavformat/ipmovie.c > @@ -91,6 +91,7 @@ typedef struct IPMVEContext { > uint32_t palette[256]; > int has_palette; > int changed; > + uint8_t send_buffer; > > unsigned int audio_bits; > unsigned int audio_channels; > @@ -154,9 +155,9 @@ static int load_ipmovie_packet(IPMVEContext *s, > AVIOContext *pb, > > } else if (s->decode_map_chunk_offset) { > > - /* send both the decode map and the video data together */ > + /* send the decode map, the video data, and the send_buffer flag > together */ > > - if (av_new_packet(pkt, 2 + s->decode_map_chunk_size + > s->video_chunk_size)) > + if (av_new_packet(pkt, 3 + s->decode_map_chunk_size + > s->video_chunk_size)) > return CHUNK_NOMEM; > > if (s->has_palette) { > @@ -178,8 +179,11 @@ static int load_ipmovie_packet(IPMVEContext *s, > AVIOContext *pb, > avio_seek(pb, s->decode_map_chunk_offset, SEEK_SET); > s->decode_map_chunk_offset = 0; > > - AV_WL16(pkt->data, s->decode_map_chunk_size); > - if (avio_read(pb, pkt->data + 2, s->decode_map_chunk_size) != > + AV_WL8(pkt->data, s->send_buffer); > + s->send_buffer = 0; > + > + AV_WL16(pkt->data + 1, s->decode_map_chunk_size); > + if (avio_read(pb, pkt->data + 3, s->decode_map_chunk_size) != > s->decode_map_chunk_size) { > av_packet_unref(pkt); > return CHUNK_EOF; > @@ -188,7 +192,7 @@ static int load_ipmovie_packet(IPMVEContext *s, > AVIOContext *pb, > avio_seek(pb, s->video_chunk_offset, SEEK_SET); > s->video_chunk_offset = 0; > > - if (avio_read(pb, pkt->data + 2 + s->decode_map_chunk_size, > + if (avio_read(pb, pkt->data + 3 + s->decode_map_chunk_size, > s->video_chunk_size) != s->video_chunk_size) { > av_packet_unref(pkt); > return CHUNK_EOF; > @@ -444,6 +448,7 @@ static int process_ipmovie_chunk(IPMVEContext *s, > AVIOContext *pb, > case OPCODE_SEND_BUFFER: > av_log(s->avf, AV_LOG_TRACE, "send buffer\n"); > avio_skip(pb, opcode_size); > + s->send_buffer = 1; > break; > > case OPCODE_AUDIO_FRAME: > @@ -590,6 +595,7 @@ static int ipmovie_read_header(AVFormatContext *s) > ipmovie->video_pts = ipmovie->audio_frame_count = 0; > ipmovie->audio_chunk_offset = ipmovie->video_chunk_offset = > ipmovie->decode_map_chunk_offset = 0; > + ipmovie->send_buffer = 0; > > /* on the first read, this will position the stream at the first chunk > */ > ipmovie->next_chunk_offset = avio_tell(pb) + 4; > -- > 2.9.4 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Wed, 2017-06-21 at 22:28 +0200, Paul B Mahol wrote: > > > Checks only for 2 bytes. > > > > > + send_buffer = AV_RL8(avpkt->data); > > + > > /* decoding map contains 4 bits of information per 8x8 block > > */ > > - s->decoding_map_size = AV_RL16(avpkt->data); > > + s->decoding_map_size = AV_RL16(avpkt->data + 1); > > Reads up to 3 bytes from input buffer. So above check needs to be > updated. Thanks for the review, I will correct this in v3 of the patchset. I'll wait to see if I get more input in the next couple of days before posting it. - HP
diff --git a/libavcodec/interplayvideo.c b/libavcodec/interplayvideo.c index df3314d..9fed8f6 100644 --- a/libavcodec/interplayvideo.c +++ b/libavcodec/interplayvideo.c @@ -990,6 +990,7 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, IpvideoContext *s = avctx->priv_data; AVFrame *frame = data; int ret; + int send_buffer; if (av_packet_get_side_data(avpkt, AV_PKT_DATA_PARAM_CHANGE, NULL)) { av_frame_unref(s->last_frame); @@ -999,8 +1000,10 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, if (buf_size < 2) return AVERROR_INVALIDDATA; + send_buffer = AV_RL8(avpkt->data); + /* decoding map contains 4 bits of information per 8x8 block */ - s->decoding_map_size = AV_RL16(avpkt->data); + s->decoding_map_size = AV_RL16(avpkt->data + 1); /* compressed buffer needs to be large enough to at least hold an entire * decoding map */ @@ -1008,9 +1011,9 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, return buf_size; - s->decoding_map = buf + 2; - bytestream2_init(&s->stream_ptr, buf + 2 + s->decoding_map_size, - buf_size - s->decoding_map_size); + s->decoding_map = buf + 3; + bytestream2_init(&s->stream_ptr, buf + 3 + s->decoding_map_size, + buf_size - s->decoding_map_size - 3); if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0) return ret; @@ -1028,7 +1031,7 @@ static int ipvideo_decode_frame(AVCodecContext *avctx, ipvideo_decode_opcodes(s, frame); - *got_frame = 1; + *got_frame = send_buffer; /* shuffle frames */ av_frame_unref(s->second_last_frame); diff --git a/libavformat/ipmovie.c b/libavformat/ipmovie.c index 29eeaf6..0705d33 100644 --- a/libavformat/ipmovie.c +++ b/libavformat/ipmovie.c @@ -91,6 +91,7 @@ typedef struct IPMVEContext { uint32_t palette[256]; int has_palette; int changed; + uint8_t send_buffer; unsigned int audio_bits; unsigned int audio_channels; @@ -154,9 +155,9 @@ static int load_ipmovie_packet(IPMVEContext *s, AVIOContext *pb, } else if (s->decode_map_chunk_offset) { - /* send both the decode map and the video data together */ + /* send the decode map, the video data, and the send_buffer flag together */ - if (av_new_packet(pkt, 2 + s->decode_map_chunk_size + s->video_chunk_size)) + if (av_new_packet(pkt, 3 + s->decode_map_chunk_size + s->video_chunk_size)) return CHUNK_NOMEM; if (s->has_palette) { @@ -178,8 +179,11 @@ static int load_ipmovie_packet(IPMVEContext *s, AVIOContext *pb, avio_seek(pb, s->decode_map_chunk_offset, SEEK_SET); s->decode_map_chunk_offset = 0; - AV_WL16(pkt->data, s->decode_map_chunk_size); - if (avio_read(pb, pkt->data + 2, s->decode_map_chunk_size) != + AV_WL8(pkt->data, s->send_buffer); + s->send_buffer = 0; + + AV_WL16(pkt->data + 1, s->decode_map_chunk_size); + if (avio_read(pb, pkt->data + 3, s->decode_map_chunk_size) != s->decode_map_chunk_size) { av_packet_unref(pkt); return CHUNK_EOF; @@ -188,7 +192,7 @@ static int load_ipmovie_packet(IPMVEContext *s, AVIOContext *pb, avio_seek(pb, s->video_chunk_offset, SEEK_SET); s->video_chunk_offset = 0; - if (avio_read(pb, pkt->data + 2 + s->decode_map_chunk_size, + if (avio_read(pb, pkt->data + 3 + s->decode_map_chunk_size, s->video_chunk_size) != s->video_chunk_size) { av_packet_unref(pkt); return CHUNK_EOF; @@ -444,6 +448,7 @@ static int process_ipmovie_chunk(IPMVEContext *s, AVIOContext *pb, case OPCODE_SEND_BUFFER: av_log(s->avf, AV_LOG_TRACE, "send buffer\n"); avio_skip(pb, opcode_size); + s->send_buffer = 1; break; case OPCODE_AUDIO_FRAME: @@ -590,6 +595,7 @@ static int ipmovie_read_header(AVFormatContext *s) ipmovie->video_pts = ipmovie->audio_frame_count = 0; ipmovie->audio_chunk_offset = ipmovie->video_chunk_offset = ipmovie->decode_map_chunk_offset = 0; + ipmovie->send_buffer = 0; /* on the first read, this will position the stream at the first chunk */ ipmovie->next_chunk_offset = avio_tell(pb) + 4;
Interplay MVE movies have a SEND_BUFFER operation. Only after this command does the current decoding buffer get displayed. This is required for the other frame formats. They are fixed-size and can't always encode a full frame worth of pixeldata. This code prevents half-finished frames from being emitted. Signed-off-by: Hein-Pieter van Braam <hp@tmm.cx> --- libavcodec/interplayvideo.c | 13 ++++++++----- libavformat/ipmovie.c | 16 +++++++++++----- 2 files changed, 19 insertions(+), 10 deletions(-)