| Message ID | 20160930163411.6743-1-matthieu.bouron@gmail.com |
|---|---|
| State | Accepted |
| Commit | 68822da8ff7dd33480c0bfdb2e769bcbe445b018 |
| Headers | show |
On 30/09/2016 17:34, Matthieu Bouron wrote: > From: Matthieu Bouron <matthieu.bouron@stupeflix.com> > > --- > libavcodec/mediacodecdec_h2645.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/mediacodecdec_h2645.c b/libavcodec/mediacodecdec_h2645.c > index 122be88..9f8110c 100644 > --- a/libavcodec/mediacodecdec_h2645.c > +++ b/libavcodec/mediacodecdec_h2645.c > @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format) > > if (pps && sps) { > uint8_t *data = NULL; > - size_t data_size = 0; > + int data_size = 0; > > if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, &data_size)) < 0) { > goto done; > Shouldn't you change h2645_ps_to_nalu()'s size arguments to size_t as well? -- Josh
On Fri, 30 Sep 2016 17:51:42 +0100 Josh de Kock <josh@itanimul.li> wrote: > On 30/09/2016 17:34, Matthieu Bouron wrote: > > From: Matthieu Bouron <matthieu.bouron@stupeflix.com> > > > > --- > > libavcodec/mediacodecdec_h2645.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/mediacodecdec_h2645.c b/libavcodec/mediacodecdec_h2645.c > > index 122be88..9f8110c 100644 > > --- a/libavcodec/mediacodecdec_h2645.c > > +++ b/libavcodec/mediacodecdec_h2645.c > > @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format) > > > > if (pps && sps) { > > uint8_t *data = NULL; > > - size_t data_size = 0; > > + int data_size = 0; > > > > if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, &data_size)) < 0) { > > goto done; > > > > Shouldn't you change h2645_ps_to_nalu()'s size arguments to size_t as well? FFmpeg uses int instead of size_t in a LOT of places (including API). Might be a little bit too late to fix this convention.
On Fri, Sep 30, 2016 at 07:01:09PM +0200, wm4 wrote: > On Fri, 30 Sep 2016 17:51:42 +0100 > Josh de Kock <josh@itanimul.li> wrote: > > > On 30/09/2016 17:34, Matthieu Bouron wrote: > > > From: Matthieu Bouron <matthieu.bouron@stupeflix.com> > > > > > > --- > > > libavcodec/mediacodecdec_h2645.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavcodec/mediacodecdec_h2645.c b/libavcodec/mediacodecdec_h2645.c > > > index 122be88..9f8110c 100644 > > > --- a/libavcodec/mediacodecdec_h2645.c > > > +++ b/libavcodec/mediacodecdec_h2645.c > > > @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format) > > > > > > if (pps && sps) { > > > uint8_t *data = NULL; > > > - size_t data_size = 0; > > > + int data_size = 0; > > > > > > if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, &data_size)) < 0) { > > > goto done; > > > > > > > Shouldn't you change h2645_ps_to_nalu()'s size arguments to size_t as well? > > FFmpeg uses int instead of size_t in a LOT of places (including API). > Might be a little bit too late to fix this convention. Yes. On the other hand: * PPS.data_size type is size_t and PPS.data is uint8_t[4096]. * ff_AMediaFormat_SetBuffer takes a size_t for the data_size argument. IMHO, I don't think switching to size_t is a real improvement here but if you think it is I can update the patch (otherwise I'll wait another day and push it as is). Matthieu
On Sat, Oct 01, 2016 at 12:52:53PM +0200, Matthieu Bouron wrote: > On Fri, Sep 30, 2016 at 07:01:09PM +0200, wm4 wrote: > > On Fri, 30 Sep 2016 17:51:42 +0100 > > Josh de Kock <josh@itanimul.li> wrote: > > > > > On 30/09/2016 17:34, Matthieu Bouron wrote: > > > > From: Matthieu Bouron <matthieu.bouron@stupeflix.com> > > > > > > > > --- > > > > libavcodec/mediacodecdec_h2645.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/libavcodec/mediacodecdec_h2645.c b/libavcodec/mediacodecdec_h2645.c > > > > index 122be88..9f8110c 100644 > > > > --- a/libavcodec/mediacodecdec_h2645.c > > > > +++ b/libavcodec/mediacodecdec_h2645.c > > > > @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format) > > > > > > > > if (pps && sps) { > > > > uint8_t *data = NULL; > > > > - size_t data_size = 0; > > > > + int data_size = 0; > > > > > > > > if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, &data_size)) < 0) { > > > > goto done; > > > > > > > > > > Shouldn't you change h2645_ps_to_nalu()'s size arguments to size_t as well? > > > > FFmpeg uses int instead of size_t in a LOT of places (including API). > > Might be a little bit too late to fix this convention. > > Yes. On the other hand: > * PPS.data_size type is size_t and PPS.data is uint8_t[4096]. > * ff_AMediaFormat_SetBuffer takes a size_t for the data_size argument. > > IMHO, I don't think switching to size_t is a real improvement here but if > you think it is I can update the patch (otherwise I'll wait another day > and push it as is). Pushed. Matthieu
diff --git a/libavcodec/mediacodecdec_h2645.c b/libavcodec/mediacodecdec_h2645.c index 122be88..9f8110c 100644 --- a/libavcodec/mediacodecdec_h2645.c +++ b/libavcodec/mediacodecdec_h2645.c @@ -154,7 +154,7 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format) if (pps && sps) { uint8_t *data = NULL; - size_t data_size = 0; + int data_size = 0; if ((ret = h2645_ps_to_nalu(sps->data, sps->data_size, &data, &data_size)) < 0) { goto done;
From: Matthieu Bouron <matthieu.bouron@stupeflix.com> --- libavcodec/mediacodecdec_h2645.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)