On 25 April 2018 at 22:22, Patrick Keroulas <
patrick.kerou...@savoirfairelinux.com> wrote:
> From: Damien Riegel <damien.rie...@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.kerou...@savoirfairelinux.com>
> Signed-off-by: Damien Riegel <damien.rie...@savoirfairelinux.com>
> ---
>
> Change v3 -> v4: cleanup
> ---
> libavcodec/avcodec.h | 8 ++++
> libavcodec/bitpacked.c | 113 ++++++++++++++++++++++++++++++
> +++++++++++++------
> 2 files changed, 107 insertions(+), 14 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
>
> enum AVSideDataParamChangeFlags {
> AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
> diff --git a/libavcodec/bitpacked.c b/libavcodec/bitpacked.c
> index 85d4bdd..380d784 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,22 +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;
>
> -
> - 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]);
> @@ -100,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;
> }
>
> @@ -131,13 +167,61 @@ static int bitpacked_decode(AVCodecContext *avctx,
> void *data, int *got_frame,
> return res;
> }
>
> - res = bc->decode(avctx, frame, avpkt);
> - if (res)
> - return res;
>
You moved the get_buffer call above here, which is okay, but you always
call it, regardless of whether there's a need for it (when there's a top
field in your context and you've just received a bottom field packet you
still call get_buffer).
You should move the get_buffer call in the if () branches and just live
with the duplicated few lines (once for when you receive top field and once
for progressive).
> + 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;
>
I think this should be handled by libavcodec/utils.c and rejected there.
Opinions from other people?
+ } 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");
>
> - *got_frame = 1;
> - return buf_size;
> + frame->interlaced_frame = 1;
> + frame->top_field_first = 1;
>
> + /* 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;
> +
> + bc->prev_top_field = 0;
> + *got_frame = 1;
> + return buf_size;
> + } else {
> + /* No field: the frame is progressive.
> + * Drop eventual top field. */
> + if (bc->prev_top_field)
> + av_frame_unref(bc->cur_interlaced_frame);
>
"eventual"? This makes no sense, the top field is already there. Just
remove the comment, its obvious from the code what's happening.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
