[FFmpeg-devel] [PATCH] avcodec: Add librav1e encoder

[Derek Buitenhuis]

On 28/05/2019 20:32, James Almer wrote:
>> +    default:
>> +        // This should be impossible
>> +        return (RaChromaSampling) -1;
> 
> If it's not meant to happen, then it should probably be an av_assert0.

Yeah, that makes more sense. Will change.

>> +    int rret;
>> +    int ret = 0;
> 
> Why two ret variables?

Will fix.

>> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>> +         const AVBitStreamFilter *filter = av_bsf_get_by_name("extract_extradata");
> 
> See
> https://github.com/dwbuiten/FFmpeg/commit/e146749232928a1cebe637338189938644237894#r33622810
> 
> You don't need to extract the Sequence Header from encoded frames since
> rav1e provides a function for this purpose.
> So does libaom, for that matter, but it's buggy and that's why it's not
> being used.

As discussed on IRC, that function does not return the config OBUs at the end of
the ISOBMFF-style data, so this way is still preferred until it does.

I'll switch over if/once rav1e's API does return that.


>> +    memcpy(pkt->data, rpkt->data, rpkt->len);
>> +
>> +    if (rpkt->frame_type == RA_FRAME_TYPE_KEY)
>> +        pkt->flags |= AV_PKT_FLAG_KEY;
>> +
>> +    pkt->pts = pkt->dts = rpkt->number * avctx->ticks_per_frame;
>> +
>> +    rav1e_packet_unref(rpkt);
> 
> You can avoid the data copy if you wrap the RaPacket into the
> AVBufferRef used in the AVPacket, but for this you need to use the
> send/receive API, given the encode2 one allows the caller to use their
> own allocated packet.

As discussed on IRC / noted in the top of this email, I'll see how
you fare switching to send/recieve. :)

>> +static const AVOption options[] = {
>> +    { "quantizer", "use constant quantizer mode", OFFSET(quantizer), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 255, VE },
>> +    { "max-quantizer", "max quantizer when using bitrate mode", OFFSET(max_quantizer), AV_OPT_TYPE_INT, { .i64 = 255 }, 1, 255, VE },
> 
> This should be mapped to qmax lavc option instead.

OK. Is there a matching option for plain old quantizer?
I couldn't find an obvious one.

- Derk