[FFmpeg-devel] [PATCH v6 1/4] lavc: add FLIF decoding support

Anamitra Ghorui aghorui at teknik.io
Thu Aug 27 06:16:00 EEST 2020


(I apologise if this mail gets sent to the wrong thread and other
errors. I'm trying out a new mail client)

On Wed, 26 Aug 2020 16:31:46 +0000
"Anamitra Ghorui" <aghorui at teknik.io> wrote:

[...]

> I agree that the case statements between if statements and while
> statements are not very good looking, but this allows us to extract the
> function logic without having to set the, or look at the state
> management variables and statements. Trying to make do without the
> interleaving statements would cause us to set s->segment many times,
> set multiple switch statements etc.
> 
>     func(FLIF16XYZContext *s, ...)
>     {
>         // Init stuff
>         ...
>         
>         switch (s->segment) {
>         /* Note how cases are right before RAC calls */
>         case 1:
>             RAC_GET(...);
>             ...
>         case 2:
>             RAC_GET(...);
>             ...
>         ...
>         }
> 
>         // End stuff
>         ...
>         return 0;
> 
>     need more data:
>         ...
>         return AVERROR(EAGAIN);
>     }
> 
> I get your point about using do {} while (0) to break the sequence, it
> does make sense now to ditch the need_more_data entirely and hence free 
> it from having to using a specific goto label, however it introduces an 
> additional level of indentation. This is how majority of the decoder 
> logic and transform logic has been written, so it may make the code 
> look a bit bulkier. Should I do it anyway?
> 
>     func(FLIF16XYZContext *s, ...)
>     {
>         // Init stuff
>         ...
> 
>         do {
>             switch (s->segment) {
>             /* Note how cases are right before RAC calls */
>             case 1:
>                 RAC_GET(...);
> 
>             case 2:
>                 RAC_GET(...);
>             ...
>             }
> 
>             // End stuff
>             ...
>             return 0;
>         } while (0);
> 
>         // EAGAIN stuff
>         ...
>         return AVERROR(EAGAIN);
>     }
> 
> 

Wait a second, that's wrong. It should be like this instead:

    int main()
    {
        // Init Stuff
        ...
        switch (k) {
        case 0:
            RAC_GET(...)
            break;
        ...
        case N:
            RAC_GET(...)

        default:
            // Success end case
            // We don't want the control here if error case
            return 0;
        }

        // Error end case
        return AVERROR(EAGAIN);
    }

--
Regards,
Anamitra



More information about the ffmpeg-devel mailing list