Hi,

On Fri, Mar 30, 2012 at 7:57 AM, Diego Biurrun <[email protected]> wrote:
> On Fri, Mar 30, 2012 at 12:53:04PM +0100, Måns Rullgård wrote:
>> Diego Biurrun <[email protected]> writes:
>> > On Fri, Mar 30, 2012 at 12:45:10PM +0100, Måns Rullgård wrote:
>> >> Diego Biurrun <[email protected]> writes:
>> >> > Also drop the now unnecessary ff_h264_ prefix from its name.
>> >> > ---
>> >> >
>> >> > --- a/libavcodec/h264.c
>> >> > +++ b/libavcodec/h264.c
>> >> > @@ -4559,7 +4559,7 @@ av_cold void ff_h264_free_context(H264Context *h)
>> >> >
>> >> > -av_cold int ff_h264_decode_end(AVCodecContext *avctx)
>> >> > +static av_cold int decode_end(AVCodecContext *avctx)
>> >>
>> >> Please keep the h264 prefix on the name.  Having somewhat unique
>> >> function names helps if they show up in a stack trace or someone wants
>> >> to set a debugger breakpoint.
>> >
>> > The problem is that we don't do this consistently anywhere, much less
>> > in this file.  Keeping the prefix on a single function looks like an
>> > exercise in futility to me.
>>
>> Two wrongs do not make a right.  Nor do three wrongs, nor 500 wrongs.
>> Even if you only get one lone right, that's still better than none at all.
>
> Two rants don't make a call for action. :)
>
> I've sent an updated patch already, but if you guys are going to insist
> on this, I would expect mentioning this topic in reviews, a documentation
> update for the new formats checklist and an attempt to fix this throughout
> the codebase.

I vote in favour of all of that.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to