On Wed, Aug 26, 2020 at 16:31:46 +0000, Anamitra Ghorui wrote:
> >> +uint8_t ff_flif16_rac_read_bit(FLIF16RangeCoder *rc,
> >> + uint8_t *target)
> >> +{
> >> + return ff_flif16_rac_get(rc, rc->range >> 1, target);
> >> +}
> >
> > If this is called often, you may want to mark it inline.
> >
>
> On Nicolas George's suggestion, I had dropped the inline keyword because
> the number of inline functions used may put stress on the memory cache.
> I think I should only inine the "higher" functions and not the "lower"
> functions like ff_flif16_rac_get in this case. Should I?
Ah, okay, I did read Nicolas's reviews, but I didn't recall this.
Please do go with his suggestions.
> 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.
I understand this justification. I just wanted to point out that the
code was not parsable to my brain, due to the macros (which I do
relaize have "goto"s in them, which is okay to me) and the "case"
labels in the middle of code blocks.
I suggest others comment on this as well.
> I get your point about using do {} while (0) to break the sequence, it
That was just a general remark regarding best practice. If it doesn't
fit into your above concept, then my comment doesn't apply.
Regards,
Moritz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
