| Message ID | 20210923162941.24977-2-cyph1984@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/mmaldec: fix pointer type warning | expand |
| 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 |
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
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".
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
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 --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;
Signed-off-by: Ho Ming Shun <cyph1984@gmail.com> --- libavcodec/mmaldec.c | 2 ++ 1 file changed, 2 insertions(+)