diff mbox series

[FFmpeg-devel,2/2] avcodec/mmaldec: fix deprecation warning

Message ID 20210923162941.24977-2-cyph1984@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avcodec/mmaldec: fix pointer type warning
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Ming Shun Ho Sept. 23, 2021, 4:29 p.m. UTC
Signed-off-by: Ho Ming Shun <cyph1984@gmail.com>
---
 libavcodec/mmaldec.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andreas Rheinhardt Sept. 23, 2021, 4:52 p.m. UTC | #1
Ho Ming Shun:
> Signed-off-by: Ho Ming Shun <cyph1984@gmail.com>
> ---
>  libavcodec/mmaldec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> index 5b75a1e74d..96140bf53d 100644
> --- a/libavcodec/mmaldec.c
> +++ b/libavcodec/mmaldec.c
> @@ -772,7 +772,9 @@ static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
>  
>      if (avctx->extradata_size && !ctx->extradata_sent) {
>          AVPacket pkt = {0};
> +FF_DISABLE_DEPRECATION_WARNINGS
>          av_init_packet(&pkt);
> +FF_ENABLE_DEPRECATION_WARNINGS
>          pkt.data = avctx->extradata;
>          pkt.size = avctx->extradata_size;
>          ctx->extradata_sent = 1;
> 

You did not fix the underlying issue (av_init_packet() is deprecated and
its use should therefore be discontinued), but just hid the warning.
This patch here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277278.html intends
to fix the underlying issue (and also the warning). I have just not
found anyone to actually test whether it works. I presume you could do
it. Would you be so kind and do it?

- Andreas

PS: There is also a second patch of this patchset:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277279.html
You can get the patches from patchwork here if that is easier for you:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=3470
Or use this command line to apply the patches directly:
curl https://patchwork.ffmpeg.org/series/3470/mbox/ | git am -3
Ming Shun Ho Sept. 23, 2021, 5:41 p.m. UTC | #2
On Fri, Sep 24, 2021 at 12:52 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Ho Ming Shun:
> > Signed-off-by: Ho Ming Shun <cyph1984@gmail.com>
> > ---
> >  libavcodec/mmaldec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> > index 5b75a1e74d..96140bf53d 100644
> > --- a/libavcodec/mmaldec.c
> > +++ b/libavcodec/mmaldec.c
> > @@ -772,7 +772,9 @@ static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
> >
> >      if (avctx->extradata_size && !ctx->extradata_sent) {
> >          AVPacket pkt = {0};
> > +FF_DISABLE_DEPRECATION_WARNINGS
> >          av_init_packet(&pkt);
> > +FF_ENABLE_DEPRECATION_WARNINGS
> >          pkt.data = avctx->extradata;
> >          pkt.size = avctx->extradata_size;
> >          ctx->extradata_sent = 1;
> >
>
> You did not fix the underlying issue (av_init_packet() is deprecated and
> its use should therefore be discontinued), but just hid the warning.

I was under the impression that it is deprecated to remove sizeof(AVPacket)
from libavcodec's ABI. Internally however it should be ok.

> This patch here:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277278.html intends
> to fix the underlying issue (and also the warning). I have just not
> found anyone to actually test whether it works. I presume you could do
> it. Would you be so kind and do it?

Had to patch it to build:

diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
index 0ea07ea787..2bed7309bb 100644
--- a/libavcodec/mmaldec.c
+++ b/libavcodec/mmaldec.c
@@ -779,7 +779,7 @@ static int ffmmal_decode(AVCodecContext *avctx,
void *data, int *got_frame,
            return ret;
    }

-    if ((ret = ffmmal_add_packet(avctx, avpkt, pkt->data, pkt->size)) < 0)
+    if ((ret = ffmmal_add_packet(avctx, avpkt, avpkt->data, avpkt->size)) < 0)
        return ret;

    if ((ret = ffmmal_fill_input_port(avctx)) < 0)

Seems to be working fine with the above patch applied.

Will this series likely be applied? I am asking because I am working on another
patch to convert this to the new dataflow API, and your patches look like they
might conflict.

>
> - Andreas
>
> PS: There is also a second patch of this patchset:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277279.html
> You can get the patches from patchwork here if that is easier for you:
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=3470
> Or use this command line to apply the patches directly:
> curl https://patchwork.ffmpeg.org/series/3470/mbox/ | git am -3
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt Sept. 23, 2021, 6:09 p.m. UTC | #3
Ming Shun Ho:
> On Fri, Sep 24, 2021 at 12:52 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
>>
>> Ho Ming Shun:
>>> Signed-off-by: Ho Ming Shun <cyph1984@gmail.com>
>>> ---
>>>  libavcodec/mmaldec.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
>>> index 5b75a1e74d..96140bf53d 100644
>>> --- a/libavcodec/mmaldec.c
>>> +++ b/libavcodec/mmaldec.c
>>> @@ -772,7 +772,9 @@ static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
>>>
>>>      if (avctx->extradata_size && !ctx->extradata_sent) {
>>>          AVPacket pkt = {0};
>>> +FF_DISABLE_DEPRECATION_WARNINGS
>>>          av_init_packet(&pkt);
>>> +FF_ENABLE_DEPRECATION_WARNINGS
>>>          pkt.data = avctx->extradata;
>>>          pkt.size = avctx->extradata_size;
>>>          ctx->extradata_sent = 1;
>>>
>>
>> You did not fix the underlying issue (av_init_packet() is deprecated and
>> its use should therefore be discontinued), but just hid the warning.
> 
> I was under the impression that it is deprecated to remove sizeof(AVPacket)
> from libavcodec's ABI. Internally however it should be ok.
> 

Some devs actually want to move AVPacket to libavutil (because it is
supposed to be cleaner) in the long term, so we already try to avoid
AVPackets on the stack in general. Note that get_packet_defaults() (the
actual successor to av_init_packet()) is internal to libavcodec/avpacket.c.

>> This patch here:
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277278.html intends
>> to fix the underlying issue (and also the warning). I have just not
>> found anyone to actually test whether it works. I presume you could do
>> it. Would you be so kind and do it?
> 
> Had to patch it to build:
> 
> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> index 0ea07ea787..2bed7309bb 100644
> --- a/libavcodec/mmaldec.c
> +++ b/libavcodec/mmaldec.c
> @@ -779,7 +779,7 @@ static int ffmmal_decode(AVCodecContext *avctx,
> void *data, int *got_frame,
>             return ret;
>     }
> 
> -    if ((ret = ffmmal_add_packet(avctx, avpkt, pkt->data, pkt->size)) < 0)
> +    if ((ret = ffmmal_add_packet(avctx, avpkt, avpkt->data, avpkt->size)) < 0)

How embarrassing. Good that it has never been applied in that form.

>         return ret;
> 
>     if ((ret = ffmmal_fill_input_port(avctx)) < 0)
> 
> Seems to be working fine with the above patch applied.
> 

Thanks for testing.

> Will this series likely be applied? I am asking because I am working on another
> patch to convert this to the new dataflow API, and your patches look like they
> might conflict.
> 

What new dataflow API? While it is very likely to lead to conflicts when
applying, I think it is unlikely to lead to actual deep conflicts that
can't be easily resolved. Do you already have a scetch of your patches
for the dataflow API for me to take a look?

- Andreas
Ming Shun Ho Sept. 23, 2021, 6:41 p.m. UTC | #4
On Fri, Sep 24, 2021 at 2:09 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Ming Shun Ho:
> > On Fri, Sep 24, 2021 at 12:52 AM Andreas Rheinhardt
> > <andreas.rheinhardt@outlook.com> wrote:
> >>
> >> Ho Ming Shun:
> >>> Signed-off-by: Ho Ming Shun <cyph1984@gmail.com>
> >>> ---
> >>>  libavcodec/mmaldec.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> >>> index 5b75a1e74d..96140bf53d 100644
> >>> --- a/libavcodec/mmaldec.c
> >>> +++ b/libavcodec/mmaldec.c
> >>> @@ -772,7 +772,9 @@ static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
> >>>
> >>>      if (avctx->extradata_size && !ctx->extradata_sent) {
> >>>          AVPacket pkt = {0};
> >>> +FF_DISABLE_DEPRECATION_WARNINGS
> >>>          av_init_packet(&pkt);
> >>> +FF_ENABLE_DEPRECATION_WARNINGS
> >>>          pkt.data = avctx->extradata;
> >>>          pkt.size = avctx->extradata_size;
> >>>          ctx->extradata_sent = 1;
> >>>
> >>
> >> You did not fix the underlying issue (av_init_packet() is deprecated and
> >> its use should therefore be discontinued), but just hid the warning.
> >
> > I was under the impression that it is deprecated to remove sizeof(AVPacket)
> > from libavcodec's ABI. Internally however it should be ok.
> >
>
> Some devs actually want to move AVPacket to libavutil (because it is
> supposed to be cleaner) in the long term, so we already try to avoid
> AVPackets on the stack in general. Note that get_packet_defaults() (the
> actual successor to av_init_packet()) is internal to libavcodec/avpacket.c.
>
> >> This patch here:
> >> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277278.html intends
> >> to fix the underlying issue (and also the warning). I have just not
> >> found anyone to actually test whether it works. I presume you could do
> >> it. Would you be so kind and do it?
> >
> > Had to patch it to build:
> >
> > diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
> > index 0ea07ea787..2bed7309bb 100644
> > --- a/libavcodec/mmaldec.c
> > +++ b/libavcodec/mmaldec.c
> > @@ -779,7 +779,7 @@ static int ffmmal_decode(AVCodecContext *avctx,
> > void *data, int *got_frame,
> >             return ret;
> >     }
> >
> > -    if ((ret = ffmmal_add_packet(avctx, avpkt, pkt->data, pkt->size)) < 0)
> > +    if ((ret = ffmmal_add_packet(avctx, avpkt, avpkt->data, avpkt->size)) < 0)
>
> How embarrassing. Good that it has never been applied in that form.
>
> >         return ret;
> >
> >     if ((ret = ffmmal_fill_input_port(avctx)) < 0)
> >
> > Seems to be working fine with the above patch applied.
> >
>
> Thanks for testing.
>
> > Will this series likely be applied? I am asking because I am working on another
> > patch to convert this to the new dataflow API, and your patches look like they
> > might conflict.
> >
>
> What new dataflow API? While it is very likely to lead to conflicts when
I meant the decoupled dataflow API.
> applying, I think it is unlikely to lead to actual deep conflicts that
> can't be easily resolved. Do you already have a scetch of your patches
> for the dataflow API for me to take a look?

Just sent out the patch here:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4935
>
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/mmaldec.c b/libavcodec/mmaldec.c
index 5b75a1e74d..96140bf53d 100644
--- a/libavcodec/mmaldec.c
+++ b/libavcodec/mmaldec.c
@@ -772,7 +772,9 @@  static int ffmmal_decode(AVCodecContext *avctx, void *data, int *got_frame,
 
     if (avctx->extradata_size && !ctx->extradata_sent) {
         AVPacket pkt = {0};
+FF_DISABLE_DEPRECATION_WARNINGS
         av_init_packet(&pkt);
+FF_ENABLE_DEPRECATION_WARNINGS
         pkt.data = avctx->extradata;
         pkt.size = avctx->extradata_size;
         ctx->extradata_sent = 1;