[FFmpeg-devel] [PATCH] avcodec/v4l2_m2m_enc: Add option to remove ivf container

[Lynne]

Apr 11, 2020, 15:56 by sw at jkqxz.net:

> On 04/04/2020 21:26, Andriy Gelman wrote:
>
>> From: Andriy Gelman <andriy.gelman at gmail.com>
>>
>> The dequeued packets from vp8 (s5p-mfc) encoder are output in ivf format
>> which breaks the stream when the packets are muxed in avformat. This commit
>> adds an option to remove the container and thus support the encoder.
>>
>> Signed-off-by: Andriy Gelman <andriy.gelman at gmail.com>
>> ---
>>  libavcodec/v4l2_m2m.h     |  2 ++
>>  libavcodec/v4l2_m2m_enc.c | 53 +++++++++++++++++++++++++++++++--------
>>  2 files changed, 45 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
>> index 456281f48c..525f9456e9 100644
>> --- a/libavcodec/v4l2_m2m.h
>> +++ b/libavcodec/v4l2_m2m.h
>> @@ -73,6 +73,8 @@ typedef struct V4L2m2mPriv {
>>  
>>  int num_output_buffers;
>>  int num_capture_buffers;
>> +    int strip_ivf;
>> +    int ivf_detected;
>>  } V4L2m2mPriv;
>>  
>>  /**
>> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
>> index c9f1741bfd..9c11f90567 100644
>> --- a/libavcodec/v4l2_m2m_enc.c
>> +++ b/libavcodec/v4l2_m2m_enc.c
>> @@ -25,6 +25,8 @@
>>  #include <sys/ioctl.h>
>>  #include <search.h>
>>  #include "libavcodec/avcodec.h"
>> +#include "libavcodec/internal.h"
>> +#include "libavutil/intreadwrite.h"
>>  #include "libavutil/pixdesc.h"
>>  #include "libavutil/pixfmt.h"
>>  #include "libavutil/opt.h"
>> @@ -256,6 +258,7 @@ static int v4l2_send_frame(AVCodecContext *avctx, const AVFrame *frame)
>>  
>>  static int v4l2_receive_packet(AVCodecContext *avctx, AVPacket *avpkt)
>>  {
>> +    V4L2m2mPriv *priv = avctx->priv_data;
>>  V4L2m2mContext *s = ((V4L2m2mPriv*)avctx->priv_data)->context;
>>  V4L2Context *const capture = &s->capture;
>>  V4L2Context *const output = &s->output;
>> @@ -281,7 +284,28 @@ static int v4l2_receive_packet(AVCodecContext *avctx, AVPacket *avpkt)
>>  }
>>  
>>  dequeue:
>> -    return ff_v4l2_context_dequeue_packet(capture, avpkt);
>> +    ret = ff_v4l2_context_dequeue_packet(capture, avpkt);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (priv->strip_ivf) {
>> +        int header_offset = 0;
>> +        if (avpkt->size >= 32 && AV_RL32(avpkt->data) == MKTAG('D','K','I','F')) {
>> +            header_offset = 32;
>> +            priv->ivf_detected = 1;
>> +        } else if (priv->ivf_detected) {
>> +            header_offset = 12;
>> +        }
>> +        header_offset = FFMIN(header_offset, avpkt->size);
>> +        avpkt->data  += header_offset;
>> +        avpkt->size  -= header_offset;
>> +
>> +        if (avpkt->size == 0) {
>>
>
> Does this case ever happen?  Wouldn't something have gone very wrong here to get here?
>
>> +            av_packet_unref(avpkt);
>> +            goto dequeue;
>> +        }
>> +    }
>> +    return 0;
>>  }
>>
>
> Could the presence of the IVF container be autodetected?  I suspect it can, because the tag will collide with the fixed start code in the intra frame at the start of the stream.  If that were possible then it would avoid having the tricky option which users are not going to easily know about.
>
> Otherwise seems ok.  It's rather horrible, but it looks like the best solution to the problem.
>

I really, really don't think its a good idea to probe the packets of encoders, so I think I'm NAK on this patch for now.
If a spec-compliant encoder cannot produce a packet that starts with DKIF, I'd be okay with it though.
Or if ivf had a CRC at the end like ogg where a match would practically guarantee its an ivf packet. Otherwise, its just too fragile.