| Message ID | 1526326150-12360-1-git-send-email-patrick.keroulas@savoirfairelinux.com |
|---|---|
| State | Superseded |
| Headers | show |
On 14 May 2018 at 20:29, Patrick Keroulas <patrick.keroulas@ savoirfairelinux.com> wrote: > From: Damien Riegel <damien.riegel@savoirfairelinux.com> > > This codec is already capable of depacking some combinations of pixel > formats and depth as defined in the RFC4175. The only difference between > progressive and interlace is that either a packet will contain the whole > frame, or only a field of the frame. > > There is no mechanism for interlaced frames reconstruction at the rtp > demux level, so it has to be handled by the codec which needs to > partially recompose an AVFrame with every incoming field AVPacket. > A frame is ouput only when the frame is completed with the 2nd field > (bottom). > > The additionnal field flags in AVPacket allow the decoder to dynamically > determine the frame format, i.e. progressive or interlaced. > > Signed-off-by: Patrick Keroulas <patrick.keroulas@savoirfairelinux.com> > Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com> > --- > > Change v4 -> v5: > Call ff_get_buffer only when necessary. > --- > libavcodec/avcodec.h | 8 ++++ > libavcodec/bitpacked.c | 127 ++++++++++++++++++++++++++++++ > ++++++++++++------- > 2 files changed, 118 insertions(+), 17 deletions(-) > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index fb0c6fa..14811be 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -1480,6 +1480,14 @@ typedef struct AVPacket { > */ > #define AV_PKT_FLAG_DISPOSABLE 0x0010 > > +/** > + * The packet contains a top field. > + */ > +#define AV_PKT_FLAG_TOP_FIELD 0x0020 > +/** > + * The packet contains a bottom field. > + */ > +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040 > Add a doc/APIchanges entry and bump lavc minor version. Also make it a separate patch. > enum AVSideDataParamChangeFlags { > AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001, > diff --git a/libavcodec/bitpacked.c b/libavcodec/bitpacked.c > index f0b417d..7c5221d 100644 > --- a/libavcodec/bitpacked.c > +++ b/libavcodec/bitpacked.c > @@ -32,8 +32,9 @@ > #include "libavutil/imgutils.h" > > struct BitpackedContext { > - int (*decode)(AVCodecContext *avctx, AVFrame *frame, > - AVPacket *pkt); > + int (*decode)(AVCodecContext *avctx, AVFrame *frame, AVPacket *pkt); > + AVFrame *cur_interlaced_frame; > + int prev_top_field; > }; > > /* For this format, it's a simple passthrough */ > @@ -60,25 +61,39 @@ static int bitpacked_decode_yuv422p10(AVCodecContext > *avctx, AVFrame *frame, > { > uint64_t frame_size = (uint64_t)avctx->width * > (uint64_t)avctx->height * 20; > 20LL will promote the multiplication to 64 bits without needing to cast both the width and the height. > uint64_t packet_size = (uint64_t)avpkt->size * 8; > + int interlaced = frame->interlaced_frame; > + int top_field = (avpkt->flags & AV_PKT_FLAG_TOP_FIELD) ? 1 : 0; > GetBitContext bc; > uint16_t *y, *u, *v; > int ret, i, j; > > - ret = ff_get_buffer(avctx, frame, 0); > - if (ret < 0) > - return ret; > - > - if (frame_size > packet_size) > + /* check packet size depending on the interlaced/progressive format */ > + if (interlaced) { > + if ((frame_size / 2) > packet_size) > + return AVERROR_INVALIDDATA; > + } else if (frame_size > packet_size) { > return AVERROR_INVALIDDATA; > + } > > if (avctx->width % 2) > return AVERROR_PATCHWELCOME; > > + /* > + * if the frame is interlaced, the avpkt we are getting is either the > top > + * or the bottom field. If it's the bottom field, it contains all the > odd > + * lines of the recomposed frame, so we start at offset 1. > + */ > + i = (interlaced && !top_field) ? 1 : 0; > + ret = init_get_bits(&bc, avpkt->data, avctx->width * avctx->height * > 20); > You already calculate width*height*20, in frame_size, reuse it. if (ret) > return ret; > > - for (i = 0; i < avctx->height; i++) { > + /* > + * Packets from interlaced frames contain either even lines, or odd > + * lines, so increment by two in that case. > + */ > + for (; i < avctx->height; i += 1 + interlaced) { > y = (uint16_t*)(frame->data[0] + i * frame->linesize[0]); > u = (uint16_t*)(frame->data[1] + i * frame->linesize[1]); > v = (uint16_t*)(frame->data[2] + i * frame->linesize[2]); > @@ -103,17 +118,35 @@ static av_cold int bitpacked_init_decoder(AVCodecContext > *avctx) > > if (avctx->codec_tag == MKTAG('U', 'Y', 'V', 'Y')) { > if (avctx->bits_per_coded_sample == 16 && > - avctx->pix_fmt == AV_PIX_FMT_UYVY422) > + avctx->pix_fmt == AV_PIX_FMT_UYVY422) { > + > + if (avctx->field_order > AV_FIELD_PROGRESSIVE) { > + av_log(avctx, AV_LOG_ERROR, "interlaced not yet supported > for 8-bit\n"); > + return AVERROR_PATCHWELCOME; > + } > + > bc->decode = bitpacked_decode_uyvy422; > - else if (avctx->bits_per_coded_sample == 20 && > - avctx->pix_fmt == AV_PIX_FMT_YUV422P10) > + } else if (avctx->bits_per_coded_sample == 20 && > + avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { > bc->decode = bitpacked_decode_yuv422p10; > - else > + } else { > return AVERROR_INVALIDDATA; > + } > } else { > return AVERROR_INVALIDDATA; > } > > + bc->cur_interlaced_frame = av_frame_alloc(); > + > + return 0; > +} > + > +static av_cold int bitpacked_end_decoder(AVCodecContext *avctx) > +{ > + struct BitpackedContext *bc = avctx->priv_data; > + > + av_frame_free(&bc->cur_interlaced_frame); > + > return 0; > } > > @@ -128,13 +161,72 @@ static int bitpacked_decode(AVCodecContext *avctx, > void *data, int *got_frame, > frame->pict_type = AV_PICTURE_TYPE_I; > frame->key_frame = 1; > > - res = bc->decode(avctx, frame, avpkt); > - if (res) > - return res; > + if ((avpkt->flags & AV_PKT_FLAG_TOP_FIELD) && > + (avpkt->flags & AV_PKT_FLAG_BOTTOM_FIELD)) { > + av_log(avctx, AV_LOG_WARNING, "Invalid field flags.\n"); > + return AVERROR_INVALIDDATA; > + } else if (avpkt->flags & AV_PKT_FLAG_TOP_FIELD) { > + if (bc->prev_top_field) > + av_log(avctx, AV_LOG_WARNING, "Consecutive top field > detected.\n"); > Drop this warning, its unneeded and will be printed if a switch or a seek happens. + > + frame->interlaced_frame = 1; > + frame->top_field_first = 1; > + > + if (avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { > Why do you check the pixfmt? This makes no sense, here or at all, its already guaranteed to be a supported pixfmt. + res = ff_get_buffer(avctx, frame, 0); > + if (res < 0) > + return res; > + } > + > + /* always decode the top (1st) field and ref the result frame > + * but don't output anything */ > + if ((res = bc->decode(avctx, frame, avpkt)) < 0) > + return res; > + > + av_frame_unref(bc->cur_interlaced_frame); > + if ((res = av_frame_ref(bc->cur_interlaced_frame, frame)) < 0) > + return res; > + > + bc->prev_top_field = 1; > + > + return 0; > + } else if (avpkt->flags & AV_PKT_FLAG_BOTTOM_FIELD) { > + if (!bc->prev_top_field) { > + av_log(avctx, AV_LOG_WARNING, "Top field missing.\n"); > + return 0; > Make it an AV_LOG_ERROR, this is more serious than a warning. also return AVERROR_INVALIDDATA; > + } > + > + frame->interlaced_frame = 1; > + frame->top_field_first = 1; > + > + /* complete the ref'd frame with bottom field and output the > + * result */ > + if ((res = bc->decode(avctx, bc->cur_interlaced_frame, avpkt)) < > 0) > + return res; > + > + if ((res = av_frame_ref(frame, bc->cur_interlaced_frame)) < 0) > + return res; > > - *got_frame = 1; > - return buf_size; > + bc->prev_top_field = 0; > + *got_frame = 1; > + return buf_size; > + } else { > + /* No field: the frame is progressive. */ > + if (bc->prev_top_field) > + av_frame_unref(bc->cur_interlaced_frame); > + > + if (avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { > Same, there's no need for a pixfmt check. This also breaks UYVY. Why did you put the check? + res = ff_get_buffer(avctx, frame, 0); > + if (res < 0) > + return res; > + } > > + if ((res = bc->decode(avctx, frame, avpkt)) < 0) > + return res; > + > + *got_frame = 1; > + return buf_size; > + } > } > > AVCodec ff_bitpacked_decoder = { > @@ -144,6 +236,7 @@ AVCodec ff_bitpacked_decoder = { > .id = AV_CODEC_ID_BITPACKED, > .priv_data_size = sizeof(struct BitpackedContext), > .init = bitpacked_init_decoder, > + .close = bitpacked_end_decoder, > .decode = bitpacked_decode, > .capabilities = AV_CODEC_CAP_EXPERIMENTAL, > }; > -- > 2.7.4 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Overall, if you fix those it should be pretty much ready.
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index fb0c6fa..14811be 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -1480,6 +1480,14 @@ typedef struct AVPacket { */ #define AV_PKT_FLAG_DISPOSABLE 0x0010 +/** + * The packet contains a top field. + */ +#define AV_PKT_FLAG_TOP_FIELD 0x0020 +/** + * The packet contains a bottom field. + */ +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040 enum AVSideDataParamChangeFlags { AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001, diff --git a/libavcodec/bitpacked.c b/libavcodec/bitpacked.c index f0b417d..7c5221d 100644 --- a/libavcodec/bitpacked.c +++ b/libavcodec/bitpacked.c @@ -32,8 +32,9 @@ #include "libavutil/imgutils.h" struct BitpackedContext { - int (*decode)(AVCodecContext *avctx, AVFrame *frame, - AVPacket *pkt); + int (*decode)(AVCodecContext *avctx, AVFrame *frame, AVPacket *pkt); + AVFrame *cur_interlaced_frame; + int prev_top_field; }; /* For this format, it's a simple passthrough */ @@ -60,25 +61,39 @@ static int bitpacked_decode_yuv422p10(AVCodecContext *avctx, AVFrame *frame, { uint64_t frame_size = (uint64_t)avctx->width * (uint64_t)avctx->height * 20; uint64_t packet_size = (uint64_t)avpkt->size * 8; + int interlaced = frame->interlaced_frame; + int top_field = (avpkt->flags & AV_PKT_FLAG_TOP_FIELD) ? 1 : 0; GetBitContext bc; uint16_t *y, *u, *v; int ret, i, j; - ret = ff_get_buffer(avctx, frame, 0); - if (ret < 0) - return ret; - - if (frame_size > packet_size) + /* check packet size depending on the interlaced/progressive format */ + if (interlaced) { + if ((frame_size / 2) > packet_size) + return AVERROR_INVALIDDATA; + } else if (frame_size > packet_size) { return AVERROR_INVALIDDATA; + } if (avctx->width % 2) return AVERROR_PATCHWELCOME; + /* + * if the frame is interlaced, the avpkt we are getting is either the top + * or the bottom field. If it's the bottom field, it contains all the odd + * lines of the recomposed frame, so we start at offset 1. + */ + i = (interlaced && !top_field) ? 1 : 0; + ret = init_get_bits(&bc, avpkt->data, avctx->width * avctx->height * 20); if (ret) return ret; - for (i = 0; i < avctx->height; i++) { + /* + * Packets from interlaced frames contain either even lines, or odd + * lines, so increment by two in that case. + */ + for (; i < avctx->height; i += 1 + interlaced) { y = (uint16_t*)(frame->data[0] + i * frame->linesize[0]); u = (uint16_t*)(frame->data[1] + i * frame->linesize[1]); v = (uint16_t*)(frame->data[2] + i * frame->linesize[2]); @@ -103,17 +118,35 @@ static av_cold int bitpacked_init_decoder(AVCodecContext *avctx) if (avctx->codec_tag == MKTAG('U', 'Y', 'V', 'Y')) { if (avctx->bits_per_coded_sample == 16 && - avctx->pix_fmt == AV_PIX_FMT_UYVY422) + avctx->pix_fmt == AV_PIX_FMT_UYVY422) { + + if (avctx->field_order > AV_FIELD_PROGRESSIVE) { + av_log(avctx, AV_LOG_ERROR, "interlaced not yet supported for 8-bit\n"); + return AVERROR_PATCHWELCOME; + } + bc->decode = bitpacked_decode_uyvy422; - else if (avctx->bits_per_coded_sample == 20 && - avctx->pix_fmt == AV_PIX_FMT_YUV422P10) + } else if (avctx->bits_per_coded_sample == 20 && + avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { bc->decode = bitpacked_decode_yuv422p10; - else + } else { return AVERROR_INVALIDDATA; + } } else { return AVERROR_INVALIDDATA; } + bc->cur_interlaced_frame = av_frame_alloc(); + + return 0; +} + +static av_cold int bitpacked_end_decoder(AVCodecContext *avctx) +{ + struct BitpackedContext *bc = avctx->priv_data; + + av_frame_free(&bc->cur_interlaced_frame); + return 0; } @@ -128,13 +161,72 @@ static int bitpacked_decode(AVCodecContext *avctx, void *data, int *got_frame, frame->pict_type = AV_PICTURE_TYPE_I; frame->key_frame = 1; - res = bc->decode(avctx, frame, avpkt); - if (res) - return res; + if ((avpkt->flags & AV_PKT_FLAG_TOP_FIELD) && + (avpkt->flags & AV_PKT_FLAG_BOTTOM_FIELD)) { + av_log(avctx, AV_LOG_WARNING, "Invalid field flags.\n"); + return AVERROR_INVALIDDATA; + } else if (avpkt->flags & AV_PKT_FLAG_TOP_FIELD) { + if (bc->prev_top_field) + av_log(avctx, AV_LOG_WARNING, "Consecutive top field detected.\n"); + + frame->interlaced_frame = 1; + frame->top_field_first = 1; + + if (avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { + res = ff_get_buffer(avctx, frame, 0); + if (res < 0) + return res; + } + + /* always decode the top (1st) field and ref the result frame + * but don't output anything */ + if ((res = bc->decode(avctx, frame, avpkt)) < 0) + return res; + + av_frame_unref(bc->cur_interlaced_frame); + if ((res = av_frame_ref(bc->cur_interlaced_frame, frame)) < 0) + return res; + + bc->prev_top_field = 1; + + return 0; + } else if (avpkt->flags & AV_PKT_FLAG_BOTTOM_FIELD) { + if (!bc->prev_top_field) { + av_log(avctx, AV_LOG_WARNING, "Top field missing.\n"); + return 0; + } + + frame->interlaced_frame = 1; + frame->top_field_first = 1; + + /* complete the ref'd frame with bottom field and output the + * result */ + if ((res = bc->decode(avctx, bc->cur_interlaced_frame, avpkt)) < 0) + return res; + + if ((res = av_frame_ref(frame, bc->cur_interlaced_frame)) < 0) + return res; - *got_frame = 1; - return buf_size; + bc->prev_top_field = 0; + *got_frame = 1; + return buf_size; + } else { + /* No field: the frame is progressive. */ + if (bc->prev_top_field) + av_frame_unref(bc->cur_interlaced_frame); + + if (avctx->pix_fmt == AV_PIX_FMT_YUV422P10) { + res = ff_get_buffer(avctx, frame, 0); + if (res < 0) + return res; + } + if ((res = bc->decode(avctx, frame, avpkt)) < 0) + return res; + + *got_frame = 1; + return buf_size; + } } AVCodec ff_bitpacked_decoder = { @@ -144,6 +236,7 @@ AVCodec ff_bitpacked_decoder = { .id = AV_CODEC_ID_BITPACKED, .priv_data_size = sizeof(struct BitpackedContext), .init = bitpacked_init_decoder, + .close = bitpacked_end_decoder, .decode = bitpacked_decode, .capabilities = AV_CODEC_CAP_EXPERIMENTAL, };