[FFmpeg-devel,v8,2/3] avcodec/bitpacked: add interlace support

Submitted by Patrick Keroulas on July 10, 2018, 9:52 p.m.

Details

Message ID 1531259521-19421-2-git-send-email-patrick.keroulas@savoirfairelinux.com
State New
Headers show

Commit Message

Patrick Keroulas July 10, 2018, 9:52 p.m.
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 AVAncillary Data carried by AVPacket side data allows 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>
---

Changes v7 -> v8: style

---
---
 libavcodec/bitpacked.c | 140 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 117 insertions(+), 23 deletions(-)

Comments

Michael Niedermayer July 11, 2018, 9:36 a.m.
On Tue, Jul 10, 2018 at 05:52:00PM -0400, Patrick Keroulas wrote:
> 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 AVAncillary Data carried by AVPacket side data allows 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>
> ---
> 
> Changes v7 -> v8: style
> 
> ---
> ---
>  libavcodec/bitpacked.c | 140 +++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 117 insertions(+), 23 deletions(-)

Do i understand correctly that this design takes data from the network
copies it then copies it again and then copies the bits again ?

IMHO you should decide if the production of rawvideo should happen in the
decoder or demuxer and then NOT mess with the data in the other lib but just
pass it without copying as far as possible.

if i look at the decoder i see in bitpacked_decode_yuv422p10() some rather
unoptimized looking bit unpack code.
And looking at rtpdec_rfc4175.c i see
"/* and now iterate over every scan lines */" and a memcpy()

With rawvideo its quite likely that you are limited by main memory speed
and 2 passes could make it half speed.
If this is the case and my quick look didnt miss something (please correct
me if i did) then this is possibly something that should be changed

thanks

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/bitpacked.c b/libavcodec/bitpacked.c
index f0b417d..e71593c 100644
--- a/libavcodec/bitpacked.c
+++ b/libavcodec/bitpacked.c
@@ -29,16 +29,18 @@ 
 #include "avcodec.h"
 #include "internal.h"
 #include "get_bits.h"
+#include "libavutil/ancillary_data.h"
 #include "libavutil/imgutils.h"
 
 struct BitpackedContext {
-    int (*decode)(AVCodecContext *avctx, AVFrame *frame,
-                  AVPacket *pkt);
+    int (*decode)(AVCodecContext *avctx, AVFrame *frame, AVPacket *pkt, uint8_t field);
+    AVFrame *cur_interlaced_frame;
+    int prev_top_field;
 };
 
 /* For this format, it's a simple passthrough */
 static int bitpacked_decode_uyvy422(AVCodecContext *avctx, AVFrame *frame,
-                                    AVPacket *avpkt)
+                                    AVPacket *avpkt, uint8_t field)
 {
     int ret;
 
@@ -56,29 +58,43 @@  static int bitpacked_decode_uyvy422(AVCodecContext *avctx, AVFrame *frame,
 }
 
 static int bitpacked_decode_yuv422p10(AVCodecContext *avctx, AVFrame *frame,
-                                      AVPacket *avpkt)
+                                      AVPacket *avpkt, uint8_t field)
 {
-    uint64_t frame_size = (uint64_t)avctx->width * (uint64_t)avctx->height * 20;
+    uint64_t frame_size = avctx->width * avctx->height * 20LL;
     uint64_t packet_size = (uint64_t)avpkt->size * 8;
+    int interlaced = frame->interlaced_frame;
+    int top_field = (field & AV_ANCILLARY_DATA_FIELD_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 ((avctx->width % 2 ) || (avctx->height % 2))
+        return AVERROR_PATCHWELCOME;
 
-    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);
+    ret = init_get_bits(&bc, avpkt->data, frame_size);
     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 +119,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;
 }
 
@@ -121,20 +155,79 @@  static int bitpacked_decode(AVCodecContext *avctx, void *data, int *got_frame,
                             AVPacket *avpkt)
 {
     struct BitpackedContext *bc = avctx->priv_data;
+    AVAncillaryData *ancillary;
     int buf_size = avpkt->size;
     AVFrame *frame = data;
-    int res;
+    int res, size;
+    uint8_t *side_data;
+    uint8_t field = AV_ANCILLARY_DATA_FIELD_NONE;
 
     frame->pict_type = AV_PICTURE_TYPE_I;
     frame->key_frame = 1;
 
-    res = bc->decode(avctx, frame, avpkt);
-    if (res)
-        return res;
+    side_data = av_packet_get_side_data(avpkt, AV_PKT_DATA_ANCILLARY, &size);
+    if (side_data) {
+          ancillary = (AVAncillaryData*)(side_data);
+          field = ancillary->field;
+    }
 
-    *got_frame = 1;
-    return buf_size;
+    if ((field & AV_ANCILLARY_DATA_FIELD_TOP_FIELD) &&
+        (field & AV_ANCILLARY_DATA_FIELD_BOTTOM_FIELD)) {
+        av_log(avctx, AV_LOG_WARNING, "Invalid field flags.\n");
+        return AVERROR_INVALIDDATA;
+    } else if (field & AV_ANCILLARY_DATA_FIELD_TOP_FIELD) {
+        frame->interlaced_frame = 1;
+        frame->top_field_first = 1;
+
+        if((res = ff_get_buffer(avctx, frame, 0)) < 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, field)) < 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 (field & AV_ANCILLARY_DATA_FIELD_BOTTOM_FIELD) {
+        if (!bc->prev_top_field) {
+            av_log(avctx, AV_LOG_ERROR, "Top field missing.\n");
+            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, field)) < 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. */
+        if (bc->prev_top_field)
+            av_frame_unref(bc->cur_interlaced_frame);
+
+        if((res = ff_get_buffer(avctx, frame, 0)) < 0)
+            return res;
+
+        if ((res = bc->decode(avctx, frame, avpkt, field)) < 0)
+            return res;
+
+        *got_frame = 1;
+        return buf_size;
+    }
 }
 
 AVCodec ff_bitpacked_decoder = {
@@ -144,6 +237,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,
 };