[FFmpeg-devel] [PATCH v1 1/2] codec: vrawdepay: add decoder for RFC4175

[Rostislav Pehlivanov]

On 23 February 2017 at 17:12, Damien Riegel <
damien.riegel at savoirfairelinux.com> wrote:

> On Wed, Feb 22, 2017 at 09:33:03PM +0000, Rostislav Pehlivanov wrote:
> > On 22 February 2017 at 20:18, Damien Riegel <
> > damien.riegel at savoirfairelinux.com> wrote:
> >
> > > On Fri, Feb 17, 2017 at 03:01:05PM -0500, Damien Riegel wrote:
> > > > Hi,
> > > >
> > > > On Thu, Feb 16, 2017 at 06:19:00PM +0000, Rostislav Pehlivanov wrote:
> > > > > >
> > > > > > No, do this in libavfilter and do not introduce another useless
> > > pseudo
> > > > > > codec
> > > > > >
> > > > >
> > > > > *libavformat, sorry
> > > >
> > > > The advantage of using a pseudo codec just to depack the stream is
> that
> > > > the input and the codec are in separate threads in ffmpeg, so it can
> > > > handle a heavier workload.
> > >
> > > Please find attached a v2, with the implementation in libavformat. Note
> > > that I don't want to send it as a patch of its own because the
> > > performance issue is not addressed.
> > >
> > > Basically, our test case is a raw input stream YUV 4:2:2 10 bits 1080p
> > > at 60fps. With the pseudo-codec, we are able to transcode it to h264
> and
> > > dump it to a file. With unpacking done in the libavformat, the input
> > > thread gets too busy and can't stand the load.
> > >
> > > In the implementation you made [1] unpacking was done in libavcodec, so
> > > why is it not an acceptable solution for mainline?
> > >
> > >
> > I now think it was ok to have a custom codec format because V210 is
> > implemented in such a way in lavc. So I think the first version of your
> > patch was better. You just didn't bother to list a valid reason besides
> > "offload it to another thread" and I didn't think of V210 at the time.
> > libavformat has no support for assembly so putting the unpacking there
> > would be slower too. I suggest posting a v3 of the patch which is like v1
> > (but please rename the codec name to something better) and which uses the
> > assembly for unpacking from the repository you linked.
>
> I'll respin a v3 with codec marked as experimental and renamed to
> something different. Which name would suit? rfc4175? rtpvideo?
>
>
I meant the AV_CODEC ID, not the demuxer, rfc4175 is perfectly fine for the
demuxer name.
I think AV_CODEC_ID_BITPACKED would be perfect. Just make sure you populate
the pixel format (e.g. to what the bitpacked should be unpacked to) in the
demuxer, and in the "decoder"/unpacker in libavcodec you just read the
pixfmt that has been set and use an appropriate unpacking function. Also
while you're at it, set everything like framerate, field order, pixel
format, etc. that you can read in the bitstream in the demuxer to allow
ffprobe and others to know what they're getting.
Keep in mind RFC4175 doesn't just support 422 YUV 10bit, LIKE MANY PEOPLE
IN BROADCASTING SEEM TO THINK BECAUSE OBVIOUSLY ITS THE GOD CHOSEN PIXEL
FORMAT, RFC4175 supports 8 bits, 10 bits, 12 bits, 16 bits as well as RGB
at all those bit depths. So for now please return AVERROR_PATCHWELCOME in
the demuxer on any currently unsupported format and please make sure that
in the future the demuxer will be able to be extended to support those
(don't assume its the "god chosen format" like those heathens like to).
Also if the demuxer receives something that isn't bitpacked (e.g. RGB/YUV
at 8 or 16 bits), it should output it directly rather than do a NOOP
"decoding"/unpacking.


For the assembly, I'd rather send it later as a separate patch, does
> that work for you?
>

Yes, that's fine.