On Sun, May 31, 2015 at 11:29:39PM +0200, Vittorio Giovara wrote:
> On Wed, May 20, 2015 at 1:21 PM, Diego Biurrun <[email protected]> wrote:
> > On Wed, May 20, 2015 at 02:08:55AM +0100, Vittorio Giovara wrote:
> >> --- a/libavcodec/internal.h
> >> +++ b/libavcodec/internal.h
> >> @@ -219,6 +219,11 @@ int ff_side_data_update_matrix_encoding(AVFrame
> >> *frame,
> >> enum AVMatrixEncoding
> >> matrix_encoding);
> >>
> >> /**
> >> + * Allocate and initialize an AV_PKT_DATA_CODING_PARAMS side data.
> >> + */
> >> +int ff_packet_default_coding_params(AVPacket *pkt);
> >
> > Please document the parameter as well, Doxygen will shout otherwise.
>
> ok, but all the others functions do not have any parameter documented
So let's not add more bugs.
> > The description makes me think that the function should maybe have
> > "init" in its name.
>
> I thought of that (or a "_get"), but can be pretty implicit imho
It's not implicit; as the function name lacks a verb it's unclear
what the function actually does.
> >> --- a/libavcodec/utils.c
> >> +++ b/libavcodec/utils.c
> >> @@ -177,6 +177,23 @@ int ff_side_data_update_matrix_encoding(AVFrame
> >> *frame,
> >>
> >> +int ff_packet_default_coding_params(AVPacket *pkt)
> >> +{
> >> + int i;
> >> + AVPacketCodingParams *params = (AVPacketCodingParams *)
> >> + av_packet_new_side_data(pkt, AV_PKT_DATA_CODING_PARAMS,
> >> + sizeof(AVPacketCodingParams));
> >
> > Indentation is off.
> >> @@ -1520,6 +1539,22 @@ int attribute_align_arg
> >> avcodec_encode_video2(AVCodecContext *avctx,
> >>
> >> + params = (AVPacketCodingParams *)
> >> + av_packet_get_side_data(avpkt, AV_PKT_DATA_CODING_PARAMS,
> >> NULL);
> >
> > indentation
>
> There is no way that line can fit 80 columns, that was the best
> disposition I could find.
We align after =, so align or move the whole lhs to the next line.
Or make it longer than 80 chars, it's not a hard rule.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
