On Mon, Mar 27, 2017 at 12:17 PM, Diego Biurrun <[email protected]> wrote:
> On Fri, Mar 24, 2017 at 04:47:55PM +0100, Vittorio Giovara wrote:
>> On Fri, Mar 24, 2017 at 4:10 PM, Diego Biurrun <[email protected]> wrote:
>> > --- a/Changelog
>> > +++ b/Changelog
>> > @@ -12,6 +12,7 @@ version <next>:
>> >  - The x86 assembler default switched from yasm to nasm, pass
>> >    --x86asmexe=yasm to configure to restore the old behavior.
>> >  - Cineform HD decoder
>> > +- FM Screen Capture Codec decoder
>>
>> FM is a tad short, would you mind using the full name Fox Magic in the
>> commit too?
>
> Changed locally.
>
>> > +static int decode_frame(AVCodecContext *avctx, void *data,
>> > +                        int *got_frame, AVPacket *avpkt)
>> > +{
>> > +    FMVCContext *s = avctx->priv_data;
>> > +    GetByteContext *gb = &s->gb;
>> > +    PutByteContext *pb = &s->pb;
>> > +    AVFrame *frame = data;
>> > +    int ret, y, x;
>> > +
>> > +    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
>>
>> nit: maybe coalesce ret declaration and initialization?
>> also maybe this could use an error message
>
> Nah.. :)
>
>> > +        return ret;
>> > +
>> > +    bytestream2_init(gb, avpkt->data, avpkt->size);
>> > +    bytestream2_skip(gb, 2);
>> > +
>> > +    frame->key_frame = !!bytestream2_get_le16(gb);
>> > +    frame->pict_type = frame->key_frame ? AV_PICTURE_TYPE_I : 
>> > AV_PICTURE_TYPE_P;
>>
>> these are usually initialized only if got_frame = 1
>
> Not sure what you want me to change here. frame->key_frame is changed
> just below ..

This was a minor suggestion, I was just thinking that you could store
!!bytestream2_get_le16(gb) in a separate variable and initialize
frame->key_frame only after *got_frame = 1.
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to