Re: [FFmpeg-devel] [PATCH] lavfi: add nlmeans_opencl filter
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > myp...@gmail.com > Sent: Monday, April 8, 2019 9:37 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add nlmeans_opencl filter > > On Mon, Apr 8, 2019 at 9:33 AM Song, Ruiling wrote: > > > > > -Original Message- > > > From: Song, Ruiling > > > Sent: Monday, April 1, 2019 3:53 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Song, Ruiling > > > Subject: [PATCH] lavfi: add nlmeans_opencl filter > > > > > > Signed-off-by: Ruiling Song > > > --- > > > This filter runs about 2x faster on integrated GPU than nlmeans on my > Skylake > > > CPU. > > > Anybody like to give some comments? > > > > Ping? > > > Tested and verified in i5-8265U Thanks for the testing. And comments about the code itself are welcome. The performance data highly depend on the research-window parameters and also the hardware. I think you may play-with the parameters to make a trade-off between speed and quality. Thanks! Ruiling > > OpenCL CPU/pocl 1.2fps with 1080P input > OpenCL GPU/intel NEO 1.2 fps with 1080P input > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
The `opencl_get_plane_format` function was incorrectly determining the
value used to set the image channel order. This resulted in all RGB
pixel formats being set to the `CL_RGBA` pixel format, regardless of
whether or not they actually *were* RGBA.
This patch fixes the issue by using the `offset` and depth of components
rather than the loop index to determine the value of `order`.
Signed-off-by: Jarek Samic
---
I have updated this patch in response to the comments on the first version.
RGB is no longer special-cased, the 2, 3, and 4 mappings to `CL_R` have been
removed, and the mapping for `CL_ARGB` has been changed to the correct value.
libavutil/hwcontext_opencl.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
index b116c5b708..593de1ca41 100644
--- a/libavutil/hwcontext_opencl.c
+++ b/libavutil/hwcontext_opencl.c
@@ -1419,8 +1419,9 @@ static int opencl_get_plane_format(enum AVPixelFormat
pixfmt,
// from the same component.
if (step && comp->step != step)
return AVERROR(EINVAL);
-order = order * 10 + c + 1;
+
depth = comp->depth;
+order = order * 10 + comp->offset / ((depth + 7) / 8) + 1;
step = comp->step;
alpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA &&
c == desc->nb_components - 1);
@@ -1456,14 +1457,11 @@ static int opencl_get_plane_format(enum AVPixelFormat
pixfmt,
case order: image_format->image_channel_order = type; break;
switch (order) {
CHANNEL_ORDER(1,CL_R);
-CHANNEL_ORDER(2,CL_R);
-CHANNEL_ORDER(3,CL_R);
-CHANNEL_ORDER(4,CL_R);
CHANNEL_ORDER(12, CL_RG);
CHANNEL_ORDER(23, CL_RG);
CHANNEL_ORDER(1234, CL_RGBA);
+CHANNEL_ORDER(2341, CL_ARGB);
CHANNEL_ORDER(3214, CL_BGRA);
-CHANNEL_ORDER(4123, CL_ARGB);
#ifdef CL_ABGR
CHANNEL_ORDER(4321, CL_ABGR);
#endif
--
2.21.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH, v3 RFC 2/2] lavc/vaapi_decode: find exact va_profile for HEVC_REXT
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Friday, April 5, 2019 00:52 > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v3 RFC 2/2] lavc/vaapi_decode: find > exact va_profile for HEVC_REXT > > On Thu, Apr 04, 2019 at 04:10:35PM +0800, Linjie Fu wrote: > > Use the profile constraint flags to determine the exact va_profile for > > HEVC_REXT. > > > > Directly cast PTLCommon to H265RawProfileTierLevel, and use > ff_h265_get_profile > > to get the exact profile. > > > > Signed-off-by: Linjie Fu > > --- > > [v2]: use constraint flags to determine the exact profile, expose the > > codec-specific stuff at the beginning. > > [v3]: move the VA version check to fix the compile issue. > > [RFC]: is it acceptable to cast PTLCommon to H265RawProfileTierLevel for > > convenience? The members in PTLCommon should be strictly matched in > > H265RawProfileTierLevel. > > > > libavcodec/vaapi_decode.c | 73 +++-- > -- > > 1 file changed, 58 insertions(+), 15 deletions(-) > > breaks build with shared libs: > > libavcodec/libavcodec.so: undefined reference to `ff_h265_get_profile' > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [ffmpeg_g] Error 1 > make: *** Waiting for unfinished jobs > libavcodec/libavcodec.so: undefined reference to `ff_h265_get_profile' > libavcodec/libavcodec.so: undefined reference to `ff_h265_get_profile' > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [ffplay_g] Error 1 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [ffprobe_g] Error 1 > > Hi Michael, Thanks for the build check and compile error messages. Actually, I also did the compile check locally and through pre-patch system. We do shared libs compile with gcc and latest libva and msdk master deps, and it passes the check. It seems this error occurs with clang. Could you provide some details on how to reproduce it? (e.g. compiler, distro, deps, flags, etc.). Thus we can add into the system to refine the pre-patch check and have this clang compile issue fixed? Thanks, Linjie ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH]lavf/movenc: Pass correct pointer to av_log().
On Mon, Apr 8, 2019 at 5:30 AM Carl Eugen Hoyos wrote: > > Hi! > > Attached patch is what git grep showed me after seeing ecdaa4b4 by Paul. > > Please comment, Carl Eugen LGTM if pass the fate. Thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi: add nlmeans_opencl filter
On Mon, Apr 8, 2019 at 9:33 AM Song, Ruiling wrote: > > > -Original Message- > > From: Song, Ruiling > > Sent: Monday, April 1, 2019 3:53 PM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Song, Ruiling > > Subject: [PATCH] lavfi: add nlmeans_opencl filter > > > > Signed-off-by: Ruiling Song > > --- > > This filter runs about 2x faster on integrated GPU than nlmeans on my > > Skylake > > CPU. > > Anybody like to give some comments? > > Ping? > Tested and verified in i5-8265U OpenCL CPU/pocl 1.2fps with 1080P input OpenCL GPU/intel NEO 1.2 fps with 1080P input ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi: add nlmeans_opencl filter
> -Original Message- > From: Song, Ruiling > Sent: Monday, April 1, 2019 3:53 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Song, Ruiling > Subject: [PATCH] lavfi: add nlmeans_opencl filter > > Signed-off-by: Ruiling Song > --- > This filter runs about 2x faster on integrated GPU than nlmeans on my Skylake > CPU. > Anybody like to give some comments? Ping? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > Cld fire > Sent: Monday, April 8, 2019 8:11 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in > `opencl_get_plane_format` > > > > > For P010, I guess that division needs to round up? > > > > Yep, rounding the division up did the trick; thanks! > > One last observation before I submit a new patch: I actually missed > previously that the order number is still not lining up for the ARGB format > (the order number that maps to ARGB in the code is currently 4123 while the > new method using the offset and depth is ending up with an order number of > 2341). Simply changing the order number that maps to ARGB from 4123 to 2341 > seems to work fine; is it okay to make that change or do the mapping > numbers need to remain exactly the same? I think we don't need to remain exact as before, you can update the case-values accordingly. And 2341 seems correct for ARGB. Thanks! Ruiling > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V1] doc/examples/metadata: fix the example can't dump FLV metadata
On Wed, Apr 3, 2019 at 3:26 PM Jun Zhao wrote:
>
> From: Jun Zhao
>
> fix the example can't dump FLV metadata.
>
> Signed-off-by: Jun Zhao
> ---
> doc/examples/metadata.c |5 +
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/doc/examples/metadata.c b/doc/examples/metadata.c
> index e330d07..b6cfa6b 100644
> --- a/doc/examples/metadata.c
> +++ b/doc/examples/metadata.c
> @@ -47,6 +47,11 @@ int main (int argc, char **argv)
> if ((ret = avformat_open_input(_ctx, argv[1], NULL, NULL)))
> return ret;
>
> +if ((ret = avformat_find_stream_info(fmt_ctx, NULL)) < 0) {
> +av_log(NULL, AV_LOG_ERROR, "Cannot find stream information\n");
> +return ret;
> +}
> +
> while ((tag = av_dict_get(fmt_ctx->metadata, "", tag,
> AV_DICT_IGNORE_SUFFIX)))
> printf("%s=%s\n", tag->key, tag->value);
>
> --
> 1.7.1
>
Pused, Thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
> -Original Message- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of > Mark Thompson > Sent: Monday, April 8, 2019 7:27 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in > `opencl_get_plane_format` > > > > This is mostly fine, but it looks like nv21, ayuv64le, and p010le are all > > having their order numbers changed to broken values. > > The changes to AYUV and NV21 both make sense - they can be supported > because the layout works, but they require special treatment to use beyond > just > taking the given planes in the order common to other formats. It doesn't seem > unreasonable to drop them because of that? I don't think any existing code > actually supports them (e.g. trying to overlay AYUV on anything is going to > mess > up totally). I think AYUV can be mapped the same as ARGB. For NV21, as OpenCL does not support CL_GR. I am ok not supporting this format unless someone strongly require this with a reason. Thanks! Ruiling > > For P010, I guess that division needs to round up? element_size = > (comp->depth > + 7) / 8. > > Thanks, > > - Mark > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V2] lavf/matroskaenc: Fix memory leak after write trailer
On Fri, Apr 5, 2019 at 12:38 AM Andreas Rheinhardt via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:
>
> Jun Zhao:
> > From: Jun Zhao
> >
> > Fix memory leak after write trailer for #7827, only store a audio
> > packet whose buffer has size greater than zero in cur_audio_pkt.
> >
> > Thanks to Andreas Rheinhardt for the suggestions.
> >
> > Signed-off-by: Jun Zhao
> > ---
> > libavformat/matroskaenc.c |3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index b9f99c4..06f3aeb 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -2534,7 +2534,8 @@ static int mkv_write_packet(AVFormatContext *s,
AVPacket *pkt)
> > // buffer an audio packet to ensure the packet containing the video
> > // keyframe's timecode is contained in the same cluster for WebM
> > if (codec_type == AVMEDIA_TYPE_AUDIO) {
> > -ret = av_packet_ref(>cur_audio_pkt, pkt);
> > +if (pkt->size > 0)
> > +ret = av_packet_ref(>cur_audio_pkt, pkt);
> > } else
> > ret = mkv_write_packet_internal(s, pkt, 0);
> > return ret;
> >
> Seems that I took quite a lot of time to write my commit message. I
> don't care which patch gets committed, but I presume you did run
> valgrind to make sure that it actually fixes #7827?
>
> - Andreas
Yes, I've run the valgrind for muxing with FLAC and other audio codec for
this issue.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
> > For P010, I guess that division needs to round up? > Yep, rounding the division up did the trick; thanks! One last observation before I submit a new patch: I actually missed previously that the order number is still not lining up for the ARGB format (the order number that maps to ARGB in the code is currently 4123 while the new method using the offset and depth is ending up with an order number of 2341). Simply changing the order number that maps to ARGB from 4123 to 2341 seems to work fine; is it okay to make that change or do the mapping numbers need to remain exactly the same? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
On 07/04/2019 23:40, Cld fire wrote: >> >> Does anything go wrong if you unconditionally add comp->offset / (depth / >> 8)? > > > Yes. Occasionally there is a depth value less than 8: > > [opencl_get_plane_format] depth is 2 < 8 for format: rgb8 on plane: 0 > [AVHWDeviceContext @ _] Format yuva420p supported. > [opencl_get_plane_format] depth is 5 < 8 for format: rgb565be on plane: 0 > [opencl_get_plane_format] depth is 5 < 8 for format: rgb565le on plane: 0 > > Which causes an FPE (divide-by-zero). If I throw in an if statement to weed > out those cases, that method of calculating the number results in the > following output: > > [AVHWDeviceContext @ _] Maximum supported image size 16384x16384. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 2 > [AVHWDeviceContext @ _] Format yuv420p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 2 > [AVHWDeviceContext @ _] Format yuv422p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 2 > [AVHWDeviceContext @ _] Format yuv444p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 2 > [AVHWDeviceContext @ _] Format yuv410p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 2 > [AVHWDeviceContext @ _] Format yuv411p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: gray, plane: 0 > [AVHWDeviceContext @ _] Format gray supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj420p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj422p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj444p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: nv12, plane: 0 > [opencl_get_plane_format] order_num: 12, pixel_format: nv12, plane: 1 > [AVHWDeviceContext @ _] Format nv12 supported. > [opencl_get_plane_format] order_num: 1, pixel_format: nv21, plane: 0 > [opencl_get_plane_format] order_num: 21, pixel_format: nv21, plane: 1 > [opencl_get_plane_format] order_num: 2341, pixel_format: argb, plane: 0 > [opencl_get_plane_format] order_num: 1234, pixel_format: rgba, plane: 0 > [AVHWDeviceContext @ _] Format rgba supported. > [opencl_get_plane_format] order_num: 4321, pixel_format: abgr, plane: 0 > [opencl_get_plane_format] order_num: 3214, pixel_format: bgra, plane: 0 > [AVHWDeviceContext @ _] Format bgra supported. > [opencl_get_plane_format] order_num: 1, pixel_format: gray16le, plane: 0 > [AVHWDeviceContext @ _] Format gray16le supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 2 > [AVHWDeviceContext @ _] Format yuv440p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj440p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 1 > [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 2 > [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 3 > [AVHWDeviceContext @ _] Format yuva420p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p16le, plane: 0 > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p16le, plane: 1 > [opencl_get_plane_format] order_num: 1,
Re: [FFmpeg-devel] [PATCH]lavf/utils: Allow url credentials to contain a slash
2019-04-03 1:19 GMT+02:00, Carl Eugen Hoyos : > 2019-03-28 19:35 GMT+01:00, Carl Eugen Hoyos : > >> Attached patch fixes ticket #7816 for me. >> >> Please review, Carl Eugen > > Ping. Please comment. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
> > Does anything go wrong if you unconditionally add comp->offset / (depth / > 8)? Yes. Occasionally there is a depth value less than 8: [opencl_get_plane_format] depth is 2 < 8 for format: rgb8 on plane: 0 [AVHWDeviceContext @ _] Format yuva420p supported. [opencl_get_plane_format] depth is 5 < 8 for format: rgb565be on plane: 0 [opencl_get_plane_format] depth is 5 < 8 for format: rgb565le on plane: 0 Which causes an FPE (divide-by-zero). If I throw in an if statement to weed out those cases, that method of calculating the number results in the following output: [AVHWDeviceContext @ _] Maximum supported image size 16384x16384. [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 2 [AVHWDeviceContext @ _] Format yuv420p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 2 [AVHWDeviceContext @ _] Format yuv422p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 2 [AVHWDeviceContext @ _] Format yuv444p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 2 [AVHWDeviceContext @ _] Format yuv410p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 2 [AVHWDeviceContext @ _] Format yuv411p supported. [opencl_get_plane_format] order_num: 1, pixel_format: gray, plane: 0 [AVHWDeviceContext @ _] Format gray supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 2 [AVHWDeviceContext @ _] Format yuvj420p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 2 [AVHWDeviceContext @ _] Format yuvj422p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 2 [AVHWDeviceContext @ _] Format yuvj444p supported. [opencl_get_plane_format] order_num: 1, pixel_format: nv12, plane: 0 [opencl_get_plane_format] order_num: 12, pixel_format: nv12, plane: 1 [AVHWDeviceContext @ _] Format nv12 supported. [opencl_get_plane_format] order_num: 1, pixel_format: nv21, plane: 0 [opencl_get_plane_format] order_num: 21, pixel_format: nv21, plane: 1 [opencl_get_plane_format] order_num: 2341, pixel_format: argb, plane: 0 [opencl_get_plane_format] order_num: 1234, pixel_format: rgba, plane: 0 [AVHWDeviceContext @ _] Format rgba supported. [opencl_get_plane_format] order_num: 4321, pixel_format: abgr, plane: 0 [opencl_get_plane_format] order_num: 3214, pixel_format: bgra, plane: 0 [AVHWDeviceContext @ _] Format bgra supported. [opencl_get_plane_format] order_num: 1, pixel_format: gray16le, plane: 0 [AVHWDeviceContext @ _] Format gray16le supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 2 [AVHWDeviceContext @ _] Format yuv440p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 2 [AVHWDeviceContext @ _] Format yuvj440p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 2 [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 3 [AVHWDeviceContext @ _] Format yuva420p supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p16le, plane: 0 [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p16le, plane: 1 [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p16le, plane: 2 [AVHWDeviceContext @ _] Format yuv420p16le supported. [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p16le, plane: 0 [opencl_get_plane_format]
Re: [FFmpeg-devel] [PATCH v2] Added XV Support
>+.long_name = NULL_IF_CONFIG_SMALL("Xunlie Video File"),
The this should
.long_name = NULL_IF_CONFIG_SMALL("Xunlei Video File”),
or
.long_name = NULL_IF_CONFIG_SMALL(“Xunlei(Thunder) Video File”),
Xunlei is Chinese PinYin[pi:n, yi:n]. Application is Thunder.app
> 在 2019年4月7日，14:53，Shivam Goyal 写道：
>
>
Thanks
Steven
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] Added XV Support
2019-04-07 8:53 GMT+02:00, Shivam Goyal :
> This time i modified the initial buffer at the time of reading
> header instead of changing the IO layer.
>
> Suggest any changes required.
Please mention ticket #3720 in the commit message.
> +static int xv_probe(const AVProbeData *p)
> +{
> +const uint8_t *d = p->buf;
> +
> +if (d[0] == 'X' &&
> +d[1] == 'L' &&
> +d[2] == 'V' &&
> +d[3] == 'F') {
> +return AVPROBE_SCORE_MAX;
Should be MAX / 2 + 1 for 32 bit match.
[...]
> +// For XV file the flv data starts from 0x20 byte
> +if(!strcmp(s->iformat->name , "xv")){
> +for (i=0; i < FFMIN(2,fileposlen); i++){
> +filepositions[i] += 0x20;
The comment does not look super useful to me.
> +static int xv_read_header(AVFormatContext *s)
This function contains trailing whitespace that cannot
be committed to the FFmpeg repository, please remove
it.
> +offset = avio_r8(ic) + rot)&0xff)<<24) |
> +(((avio_r8(ic) + rot)&0xff)<<16) |
> +(((avio_r8(ic) + rot)&0xff)<<8) |
> +(((avio_r8(ic) + rot)&0xff)))+ 0x20;
Too many parentheses.
> +// Will modify the current buffer, as only
I don't know if this is something a demuxer is allowed to do,
hopefully another developer will comment.
> +// the bytes from 0x20 to 0x200400 are needed to decode
> +int size = ic->buf_end - ic->buf_ptr;
> +int64_t pos = ic->pos - size;
> +for(int i=0;i<0x400; i++){
Maybe I miss something but it looks as if only one of i and
pos should be needed, no? (And size could be merged as
well if that does not make reading the function too difficult.)
In any case, formatting should be:
for (int x = 0; x < 0x123; i++) {
> +if(pos >= 0x200400) break;
> +(*(ic->buf_ptr + i)) = (((*(ic->buf_ptr + i)) + rot) & 0xff );
Why is this not "ic->buf_ptr[i] = ic->buf_ptr[..."?
And assuming this is a char*, I would suspect the "& 0xff" is
not necessary.
+pos+=1;
pos++
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] avcodec: add AV1 frame split bitstream filter
On 3/31/2019 6:05 PM, James Almer wrote: > On 3/26/2019 9:25 PM, James Almer wrote: >> This will be needed by the eventual native AV1 decoder. >> >> Signed-off-by: James Almer >> --- >> Now propagating the Temporal Unit unchanged if splitting can't be performed. > > Ping. I'll push this soon. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH]lavf/movenc: Pass correct pointer to av_log().
Hi!
Attached patch is what git grep showed me after seeing ecdaa4b4 by Paul.
Please comment, Carl Eugen
From 564a27a19c930c73cb00b4ac013f26e9417a5a57 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos
Date: Sun, 7 Apr 2019 23:28:00 +0200
Subject: [PATCH] lavf/movenc: Pass correct pointer to av_log().
---
libavformat/movenc.c | 52 --
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index c67f909..46d314f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -313,7 +313,7 @@ static int mov_write_amr_tag(AVIOContext *pb, MOVTrack *track)
return 0x11;
}
-static int mov_write_ac3_tag(AVIOContext *pb, MOVTrack *track)
+static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
{
GetBitContext gbc;
PutBitContext pbc;
@@ -321,7 +321,7 @@ static int mov_write_ac3_tag(AVIOContext *pb, MOVTrack *track)
int fscod, bsid, bsmod, acmod, lfeon, frmsizecod;
if (track->vos_len < 7) {
-av_log(pb, AV_LOG_ERROR,
+av_log(s, AV_LOG_ERROR,
"Cannot write moov atom before AC3 packets."
" Set the delay_moov flag to fix this.\n");
return AVERROR(EINVAL);
@@ -535,7 +535,7 @@ end:
}
#endif
-static int mov_write_eac3_tag(AVIOContext *pb, MOVTrack *track)
+static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
{
PutBitContext pbc;
uint8_t *buf;
@@ -543,7 +543,7 @@ static int mov_write_eac3_tag(AVIOContext *pb, MOVTrack *track)
int size, i;
if (!track->eac3_priv) {
-av_log(pb, AV_LOG_ERROR,
+av_log(s, AV_LOG_ERROR,
"Cannot write moov atom before EAC3 packets parsed.\n");
return AVERROR(EINVAL);
}
@@ -748,14 +748,14 @@ static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track)
return update_size(pb, pos);
}
-static int mov_write_dops_tag(AVIOContext *pb, MOVTrack *track)
+static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
{
int64_t pos = avio_tell(pb);
avio_wb32(pb, 0);
ffio_wfourcc(pb, "dOps");
avio_w8(pb, 0); /* Version */
if (track->par->extradata_size < 19) {
-av_log(pb, AV_LOG_ERROR, "invalid extradata size\n");
+av_log(s, AV_LOG_ERROR, "invalid extradata size\n");
return AVERROR_INVALIDDATA;
}
/* extradata contains an Ogg OpusHead, other than byte-ordering and
@@ -825,9 +825,9 @@ static int mov_write_wave_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
} else if (track->par->codec_id == AV_CODEC_ID_AMR_NB) {
mov_write_amr_tag(pb, track);
} else if (track->par->codec_id == AV_CODEC_ID_AC3) {
-mov_write_ac3_tag(pb, track);
+mov_write_ac3_tag(s, pb, track);
} else if (track->par->codec_id == AV_CODEC_ID_EAC3) {
-mov_write_eac3_tag(pb, track);
+mov_write_eac3_tag(s, pb, track);
} else if (track->par->codec_id == AV_CODEC_ID_ALAC ||
track->par->codec_id == AV_CODEC_ID_QDM2) {
mov_write_extradata_tag(pb, track);
@@ -1134,9 +1134,9 @@ static int mov_write_audio_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
else if (track->par->codec_id == AV_CODEC_ID_AMR_NB)
ret = mov_write_amr_tag(pb, track);
else if (track->par->codec_id == AV_CODEC_ID_AC3)
-ret = mov_write_ac3_tag(pb, track);
+ret = mov_write_ac3_tag(s, pb, track);
else if (track->par->codec_id == AV_CODEC_ID_EAC3)
-ret = mov_write_eac3_tag(pb, track);
+ret = mov_write_eac3_tag(s, pb, track);
else if (track->par->codec_id == AV_CODEC_ID_ALAC)
ret = mov_write_extradata_tag(pb, track);
else if (track->par->codec_id == AV_CODEC_ID_WMAPRO)
@@ -1144,7 +1144,7 @@ static int mov_write_audio_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContex
else if (track->par->codec_id == AV_CODEC_ID_FLAC)
ret = mov_write_dfla_tag(pb, track);
else if (track->par->codec_id == AV_CODEC_ID_OPUS)
-ret = mov_write_dops_tag(pb, track);
+ret = mov_write_dops_tag(s, pb, track);
else if (track->vos_len > 0)
ret = mov_write_glbl_tag(pb, track);
@@ -1691,12 +1691,12 @@ static int mov_write_subtitle_tag(AVIOContext *pb, MOVTrack *track)
return update_size(pb, pos);
}
-static int mov_write_st3d_tag(AVIOContext *pb, AVStereo3D *stereo_3d)
+static int mov_write_st3d_tag(AVFormatContext *s, AVIOContext *pb, AVStereo3D *stereo_3d)
{
int8_t stereo_mode;
if (stereo_3d->flags != 0) {
-av_log(pb, AV_LOG_WARNING, "Unsupported stereo_3d flags %x. st3d not written.\n", stereo_3d->flags);
+av_log(s, AV_LOG_WARNING, "Unsupported stereo_3d flags %x. st3d not written.\n", stereo_3d->flags);
return 0;
}
@@ -1711,7 +1711,7 @@ static int mov_write_st3d_tag(AVIOContext *pb, AVStereo3D *stereo_3d)
stereo_mode = 2;
Re: [FFmpeg-devel] [PATCH] libavutil/hwcontext_opencl.c: fix bug in `opencl_get_plane_format`
On 06/04/2019 00:05, Jarek Samic wrote: > The `opencl_get_plane_format` function was incorrectly determining the > value used to set the image channel order. This resulted in all RGB > pixel formats being set to the `CL_RGBA` pixel format, regardless of > whether or not they actually *were* RGBA. > > This patch fixes the issue by using the `offset` field on components > rather than the loop index to determine the value of `order` for RGB > pixel formats (and leaves the formula to determine `order` the same for > other formats so as not to break those cases). > > Signed-off-by: Jarek Samic > --- > I'm including this in the email (but not the commit description) to > make this patch easier to review. In order to make sure I was fixing > the bug and not messing up channel order mapping for any of the other > formats, I set up a few quick print statements within the function > after the `order` value was set. > > Here's the output without this patch: > > ``` > [AVHWDeviceContext @ _] Maximum supported image size 16384x16384. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv420p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv420p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv420p, plane: 2 > [AVHWDeviceContext @ _] Format yuv420p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv422p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv422p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv422p, plane: 2 > [AVHWDeviceContext @ _] Format yuv422p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv444p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv444p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv444p, plane: 2 > [AVHWDeviceContext @ _] Format yuv444p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv410p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv410p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv410p, plane: 2 > [AVHWDeviceContext @ _] Format yuv410p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv411p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv411p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv411p, plane: 2 > [AVHWDeviceContext @ _] Format yuv411p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: gray, plane: 0 > [AVHWDeviceContext @ _] Format gray supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj420p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuvj420p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuvj420p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj420p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj422p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuvj422p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuvj422p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj422p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj444p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuvj444p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuvj444p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj444p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: nv12, plane: 0 > [opencl_get_plane_format] order_num: 23, pixel_format: nv12, plane: 1 > [AVHWDeviceContext @ _] Format nv12 supported. > [opencl_get_plane_format] order_num: 1, pixel_format: nv21, plane: 0 > [opencl_get_plane_format] order_num: 23, pixel_format: nv21, plane: 1 > [AVHWDeviceContext @ _] Format nv21 supported. > [opencl_get_plane_format] order_num: 1234, pixel_format: argb, plane: 0 > [AVHWDeviceContext @ _] Format argb supported. > [opencl_get_plane_format] order_num: 1234, pixel_format: rgba, plane: 0 > [AVHWDeviceContext @ _] Format rgba supported. > [opencl_get_plane_format] order_num: 1234, pixel_format: abgr, plane: 0 > [AVHWDeviceContext @ _] Format abgr supported. > [opencl_get_plane_format] order_num: 1234, pixel_format: bgra, plane: 0 > [AVHWDeviceContext @ _] Format bgra supported. > [opencl_get_plane_format] order_num: 1, pixel_format: gray16le, plane: 0 > [AVHWDeviceContext @ _] Format gray16le supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuv440p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuv440p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuv440p, plane: 2 > [AVHWDeviceContext @ _] Format yuv440p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuvj440p, plane: 0 > [opencl_get_plane_format] order_num: 2, pixel_format: yuvj440p, plane: 1 > [opencl_get_plane_format] order_num: 3, pixel_format: yuvj440p, plane: 2 > [AVHWDeviceContext @ _] Format yuvj440p supported. > [opencl_get_plane_format] order_num: 1, pixel_format: yuva420p, plane: 0 >
Re: [FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
On 4/7/2019 4:22 PM, Marton Balint wrote: > > > On Sun, 7 Apr 2019, James Almer wrote: > >> On 4/7/2019 3:47 PM, Marton Balint wrote: >>> framethread.c is put into libavutil, but is has to be included >>> directly to >>> avoid creating avpriv functions. >> >> If the reason behind this factorization is sharing the code between >> modules across several libraries and the function signatures are >> unlikely to change, then using avpriv_ functions is justified. >> The structs will be internal to libavutil, so the only thing tied to the >> ABI will be the avpriv symbols. >> >> Including the entire c file will result in unnecessary binary bloat once >> this code is used for example in libavfilter. > > Yeah, I agree. I just did not want to make it avpriv until we have at > least a second component using it, because I cannot forsee if the API > will be sufficent or not. Fair enough, guess it should be ok like this until it's needed elsewhere, to avoid having to potentially live with useless symbols until a major bump if they ultimately need to be changed. Although i have to say it's kinda weird having a source file in the libavutil folder that's not compiled into said library. Wait to see if someone else comments. > > Regards, > Marton > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/get_bits: unbreak get_bits_le() with cached reader
On 3/26/19, Paul B Mahol wrote: > On 3/26/19, Carl Eugen Hoyos wrote: >> 2019-03-26 11:34 GMT+01:00, Paul B Mahol : >>> On 3/26/19, Carl Eugen Hoyos wrote: 2019-03-26 11:13 GMT+01:00, Paul B Mahol : > Signed-off-by: Paul B Mahol > --- > libavcodec/get_bits.h | 82 > + > libavcodec/utvideodec.c | 4 +- > 2 files changed, 60 insertions(+), 26 deletions(-) How can the issue be reproduced? Would my patch - that does not increase the number of lines in the source tree - also fix the issue? >>> >>> I never saw your patch so answer is no. >> >> No, you don't know how to reproduce the issue? >> >> You gave a very extensive comment on the patch: >> https://patchwork.ffmpeg.org/patch/4784/ > > This is about using both endianess versions from same file. > That patch changes that for usage of single endianess in file. > Going to apply it soon. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
On Sun, 7 Apr 2019, James Almer wrote: On 4/7/2019 3:47 PM, Marton Balint wrote: framethread.c is put into libavutil, but is has to be included directly to avoid creating avpriv functions. If the reason behind this factorization is sharing the code between modules across several libraries and the function signatures are unlikely to change, then using avpriv_ functions is justified. The structs will be internal to libavutil, so the only thing tied to the ABI will be the avpriv symbols. Including the entire c file will result in unnecessary binary bloat once this code is used for example in libavfilter. Yeah, I agree. I just did not want to make it avpriv until we have at least a second component using it, because I cannot forsee if the API will be sufficent or not. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCHv2] avformat/file: add seekable option to disallow seeking
v2: Disallow positive override of seekability as requested by Hendrik Leppkes.
Signed-off-by: Marton Balint
---
doc/protocols.texi | 8
libavformat/file.c | 5 +
2 files changed, 13 insertions(+)
diff --git a/doc/protocols.texi b/doc/protocols.texi
index e1caa049a5..3e4e7af3d4 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -199,6 +199,14 @@ If set to 1, the protocol will retry reading at the end of
the file, allowing
reading files that still are being written. In order for this to terminate,
you either need to use the rw_timeout option, or use the interrupt callback
(for API users).
+
+@item seekable
+Controls if seekability is advertised on the file. 0 means non-seekable, -1
+means auto (seekable for normal files, non-seekable for named pipes).
+
+Many demuxers handle seekable and non-seekable resources differently,
+overriding this might speed up opening certain files at the cost of losing some
+features (e.g. accurate seeking).
@end table
@section ftp
diff --git a/libavformat/file.c b/libavformat/file.c
index e613b91010..08c7f8e6dd 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -73,6 +73,7 @@ typedef struct FileContext {
int trunc;
int blocksize;
int follow;
+int seekable;
#if HAVE_DIRENT_H
DIR *dir;
#endif
@@ -82,6 +83,7 @@ static const AVOption file_options[] = {
{ "truncate", "truncate existing files on write", offsetof(FileContext,
trunc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, AV_OPT_FLAG_ENCODING_PARAM },
{ "blocksize", "set I/O operation maximum block size",
offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1,
INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
{ "follow", "Follow a file as it is being written", offsetof(FileContext,
follow), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
+{ "seekable", "Sets if the file is seekable", offsetof(FileContext,
seekable), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 0, AV_OPT_FLAG_DECODING_PARAM |
AV_OPT_FLAG_ENCODING_PARAM },
{ NULL }
};
@@ -238,6 +240,9 @@ static int file_open(URLContext *h, const char *filename,
int flags)
if (!h->is_streamed && flags & AVIO_FLAG_WRITE)
h->min_packet_size = h->max_packet_size = 262144;
+if (c->seekable >= 0)
+h->is_streamed = !c->seekable;
+
return 0;
}
--
2.16.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
On 4/7/2019 3:47 PM, Marton Balint wrote: > framethread.c is put into libavutil, but is has to be included directly to > avoid creating avpriv functions. If the reason behind this factorization is sharing the code between modules across several libraries and the function signatures are unlikely to change, then using avpriv_ functions is justified. The structs will be internal to libavutil, so the only thing tied to the ABI will be the avpriv symbols. Including the entire c file will result in unnecessary binary bloat once this code is used for example in libavfilter. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
On Sun, 7 Apr 2019, Paul B Mahol wrote: On 4/7/19, Marton Balint wrote: framethread.c is put into libavutil, but is has to be included directly to avoid creating avpriv functions. Functionality should be identical, there is one slight difference: we close the per-thread avcodec contexts in the main thread and not in the workers. Why? Factorization is good because threading becomes more readable and can be reused later in e.g. filter frame threading. Per-thread contexts are closed and freed in the main thread because doing it in the worker would have required another callback in the thread context, and it seems pointless to do it in the workers because if you check the current code, avcodec_close is serialized using a mutex. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
On 4/7/19, Marton Balint wrote: > framethread.c is put into libavutil, but is has to be included directly to > avoid creating avpriv functions. > > Functionality should be identical, there is one slight difference: we close > the > per-thread avcodec contexts in the main thread and not in the workers. > Why? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 00/21] New Version
Hello, thanks for taking the time to review this. Much appreciated. Steve Lhomme: > Hi, > > On 3/27/2019 12:18 PM, Andreas Rheinhardt wrote: >> This also changed the handling of unknown-sized elements: They are now >> ended whenever an element not known to be allowed in them is >> encountered. If we are already on level 1 and encounter an element not >> known to be allowed in an unknown-sized segment, this is treated as an >> indication that an error might have occured. I hope this is fine. > > I haven't looked at the code yet, but an unknown element doesn't mean > it's an upper level element. I think it should be either skipped or > considered as bad data. If it's a known element but complitely > misplaced it should not be going upper in the hierarchy. Only when a > valid upper element it should go up in the hierarchy. > 1. Actually the current approach is equivalent to considering unknown elements in an unknown-sized element as bad data: If a new ID isn't in the current syntax list, then we treat this as an indication that the current level ended and test with the next higher level and so on until we arrive at level 1. If it then isn't a valid level 1 element, then it is being treated as an error. 2. But as you have said (in your last mail), this approach won't work with extensions like RAWCooked. So a check for whether an element is from a higher level needs to be implemented and unknown elements with finite size should be skipped. 3. But there are practical complications: a) If an error occurred, then it is possible that we encounter garbage that looks like a very big unknown element. I'd image that trying to skip it might very well lead to problems, in particular in case of livestreaming. So there should be a sanity check for this, i.e. very big unknown elements should be considered as errors. b) And it is also possible that we encounter quite a few possibly legal unknown elements (that are skipped) in a row. I think it's reasonable to view this as an indication that an error occurred, too. How long should these chains of unknown elements be for it to trigger this error? Three? Four? Five? c) In both cases, resyncing should begin at the last known good position, not at the current position. Do you agree with me regarding 2. and 3.? 4. Would it actually be legal for an extension like RAWCooked to implement new potentially unknown-sized elements? I haven't found anything on this topic that says it is illegal, so I presume it is legal. But it shouldn't be IMO if it wants to be an extension that can be played by normal players (that don't know these extra elements), as unknown-sized elements simply can't be skipped. I wouldn't know how to handle them other than resyncing to the next known level 1 element, so it would be nice not to have to worry about unknown master elements with unknown size. >> >> Dale's sample "bear-320x240-live.webm" btw has cues at the end that use >> unknown-sized elements (wastefully coded on eight bytes) for >> CuePoints and >> CueTrackPositions which is spec-incompliant. They are not referenced by >> a SeekHead and so can't be used for seeking, but if they were, neither >> current FFmpeg nor FFmpeg with my patches applied would be able to use >> them. Are such files common (this is the first such file I ever saw)? >> If so, I could probably make it work. > > If CuePoints are not referenced by the SeekHead at the front of the > file (or the indirect SeekHead) they are useless anyway. > I know, but should I try to support unknown-sized elements other than segments and cluster at all? >> >> I have cced Steve for this (I didn't the first time, because I >> thought that he (as a maintainer) would also be a subscriber to this >> list). > > I am subscribed but not the maintainer. In fact I know little about > this code. > I didn't want to say that you are maintainer of the Matroska demuxer; but you are listed as maintainer for certain hw video accelerations stuff. >> Oh, and I did not check with Valgrind that the new lacing code doesn't >> read uninitialized data. I don't even know how to use Valgrind. I just >> read the code. If someone more knowledgeable than I could please test >> it... > > You might also use LLVM with ASAN (address sanitizer) it's helpfull > for this (or so I have been told). Windows 7 not supported. - 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/frame_thread_encoder: factorize frame threading
framethread.c is put into libavutil, but is has to be included directly to
avoid creating avpriv functions.
Functionality should be identical, there is one slight difference: we close the
per-thread avcodec contexts in the main thread and not in the workers.
Signed-off-by: Marton Balint
---
libavcodec/frame_thread_encoder.c | 229 ++
libavutil/framethread.c | 207 ++
2 files changed, 288 insertions(+), 148 deletions(-)
create mode 100644 libavutil/framethread.c
diff --git a/libavcodec/frame_thread_encoder.c
b/libavcodec/frame_thread_encoder.c
index 55756c4c54..72b5d72803 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -22,7 +22,6 @@
#include "frame_thread_encoder.h"
-#include "libavutil/fifo.h"
#include "libavutil/avassert.h"
#include "libavutil/imgutils.h"
#include "libavutil/opt.h"
@@ -31,93 +30,67 @@
#include "internal.h"
#include "thread.h"
-#define MAX_THREADS 64
-#define BUFFER_SIZE (2*MAX_THREADS)
+#include "libavutil/framethread.c"
-typedef struct{
-void *indata;
-void *outdata;
-int64_t return_code;
-unsigned index;
-} Task;
-
-typedef struct{
+typedef struct {
+AVCodecContext *thread_avctx[FRAMETHREAD_MAX_THREADS];
AVCodecContext *parent_avctx;
pthread_mutex_t buffer_mutex;
+FrameThread *framethread;
+} EncodeThreadContext;
+
+typedef struct {
+EncodeThreadContext *ctx;
+AVFrame *frame;
+AVPacket *packet;
+} EncodeTaskContext;
+
+static void encode_task_free(void *task_priv)
+{
+EncodeTaskContext *task = task_priv;
+av_frame_free(>frame);
+av_packet_free(>packet);
+}
-AVFifoBuffer *task_fifo;
-pthread_mutex_t task_fifo_mutex;
-pthread_cond_t task_fifo_cond;
-
-Task finished_tasks[BUFFER_SIZE];
-pthread_mutex_t finished_task_mutex;
-pthread_cond_t finished_task_cond;
-
-unsigned task_index;
-unsigned finished_task_index;
-
-pthread_t worker[MAX_THREADS];
-atomic_int exit;
-} ThreadContext;
-
-static void * attribute_align_arg worker(void *v){
-AVCodecContext *avctx = v;
-ThreadContext *c = avctx->internal->frame_thread_encoder;
-AVPacket *pkt = NULL;
-
-while (!atomic_load(>exit)) {
-int got_packet, ret;
-AVFrame *frame;
-Task task;
-
-if(!pkt) pkt = av_packet_alloc();
-if(!pkt) continue;
-av_init_packet(pkt);
-
-pthread_mutex_lock(>task_fifo_mutex);
-while (av_fifo_size(c->task_fifo) <= 0 || atomic_load(>exit)) {
-if (atomic_load(>exit)) {
-pthread_mutex_unlock(>task_fifo_mutex);
-goto end;
-}
-pthread_cond_wait(>task_fifo_cond, >task_fifo_mutex);
-}
-av_fifo_generic_read(c->task_fifo, , sizeof(task), NULL);
-pthread_mutex_unlock(>task_fifo_mutex);
-frame = task.indata;
+static int encode_task_func(void *task_priv, int thread_id)
+{
+EncodeTaskContext *task = task_priv;
+EncodeThreadContext *c = task->ctx;
+AVPacket *pkt = av_packet_alloc();
+int got_packet, ret;
-ret = avcodec_encode_video2(avctx, pkt, frame, _packet);
+if (!pkt) {
pthread_mutex_lock(>buffer_mutex);
-av_frame_unref(frame);
+av_frame_unref(task->frame);
pthread_mutex_unlock(>buffer_mutex);
-av_frame_free();
-if(got_packet) {
-int ret2 = av_packet_make_refcounted(pkt);
-if (ret >= 0 && ret2 < 0)
-ret = ret2;
-} else {
-pkt->data = NULL;
-pkt->size = 0;
-}
-pthread_mutex_lock(>finished_task_mutex);
-c->finished_tasks[task.index].outdata = pkt; pkt = NULL;
-c->finished_tasks[task.index].return_code = ret;
-pthread_cond_signal(>finished_task_cond);
-pthread_mutex_unlock(>finished_task_mutex);
+av_frame_free(>frame);
+return AVERROR(ENOMEM);
}
-end:
-av_free(pkt);
+
+av_init_packet(pkt);
+
+ret = avcodec_encode_video2(c->thread_avctx[thread_id], pkt, task->frame,
_packet);
pthread_mutex_lock(>buffer_mutex);
-avcodec_close(avctx);
+av_frame_unref(task->frame);
pthread_mutex_unlock(>buffer_mutex);
-av_freep();
-return NULL;
+av_frame_free(>frame);
+if(got_packet) {
+int ret2 = av_packet_make_refcounted(pkt);
+if (ret >= 0 && ret2 < 0)
+ret = ret2;
+} else {
+pkt->data = NULL;
+pkt->size = 0;
+}
+task->packet = pkt;
+
+return ret;
}
int ff_frame_thread_encoder_init(AVCodecContext *avctx, AVDictionary *options){
int i=0;
-ThreadContext *c;
-
+int ret;
+EncodeThreadContext *c;
if( !(avctx->thread_type & FF_THREAD_FRAME)
|| !(avctx->codec->capabilities & AV_CODEC_CAP_INTRA_ONLY))
@@ -164,32 +137,29 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx,
Re: [FFmpeg-devel] [PATCH 18/21] avformat/matroskadec: Combine two arrays
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> By including SimpleBlocks and Blocksgroups twice in the same EbmlSyntax
>> array (with different semantics), one can reduce the duplication of the
>> other values.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 13 +++--
>> 1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 3adcb3e86d..60f58cefa9 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -704,25 +704,18 @@ static const EbmlSyntax matroska_blockgroup[] = {
>> };
>> static const EbmlSyntax matroska_cluster_parsing[] = {
>> - { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,
>> offsetof(MatroskaCluster, timecode) },
>> - { MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n =
>> matroska_blockgroup } },
>> { MATROSKA_ID_SIMPLEBLOCK, EBML_BIN, 0,
>> offsetof(MatroskaBlock, bin) },
>> - { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
>> - { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
>> - { 0 }
>> -};
>> -
>> -static const EbmlSyntax matroska_cluster_initial[] = {
>> + { MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n =
>> matroska_blockgroup } },
>> { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,
>> offsetof(MatroskaCluster, timecode) },
>> - { MATROSKA_ID_BLOCKGROUP, EBML_STOP },
>> { MATROSKA_ID_SIMPLEBLOCK, EBML_STOP },
>> + { MATROSKA_ID_BLOCKGROUP, EBML_STOP },
>> { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
>> { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
>> { 0 }
>> };
>> static const EbmlSyntax matroska_cluster_enter[] = {
>> - { MATROSKA_ID_CLUSTER, EBML_NEST, 0, 0, { .n =
>> matroska_cluster_initial } },
>> + { MATROSKA_ID_CLUSTER, EBML_NEST, 0, 0, { .n =
>> _cluster_parsing[2] } },
>
> To avoid breaking this optimisation when the code is changed you might
> use some static_assert to make sure that
> matroska_cluster_parsing[2].id is MATROSKA_ID_CLUSTERTIMECODE
static_assert is actually C11, not C90 (as FFmpeg is), so I think a
comment should suffice.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/21] avformat/matroskadec: Redo level handling
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> This commit changes how levels are handled: If the level used for
>> ebml_parse ends directly after an element that has been consumed, then
>> ebml_parse ends the level itself (and any finite-sized levels that end
>> there as well) and informs the caller via the return value; if the
>> current level is unknown-sized, then the level is ended as soon as
>> an element that is not valid on the current level is encountered.
>>
>> This is designed for situations where one wants to parse master
>> elements
>> incrementally, i.e. not in one go via ebml_parse_nest.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 104
>> +++---
>> 1 file changed, 85 insertions(+), 19 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 42f1c21042..32cf57685f 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -69,6 +69,8 @@
>> #include "qtpalette.h"
>> #define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length,
>> in uint64_t */
>> +#define LEVEL_ENDED 2 /* return value of
>> ebml_parse when the
>> + * syntax level used for
>> parsing ended. */
>> typedef enum {
>> EBML_NONE,
>> @@ -1041,16 +1043,32 @@ static int
>> ebml_parse_id(MatroskaDemuxContext *matroska, EbmlSyntax *syntax,
>> uint32_t id, void *data)
>> {
>> int i;
>> +
>> for (i = 0; syntax[i].id; i++)
>> if (id == syntax[i].id)
>> break;
>> - if (!syntax[i].id && id == MATROSKA_ID_CLUSTER &&
>> - matroska->num_levels > 0 &&
>> - matroska->levels[matroska->num_levels - 1].length ==
>> EBML_UNKNOWN_LENGTH)
>> - return 0; // we reached the end of an unknown size cluster
>> +
>> if (!syntax[i].id && id != EBML_ID_VOID && id != EBML_ID_CRC32) {
>> - av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry
>> 0x%"PRIX32"\n", id);
>> + if (!matroska->num_levels ||
>> matroska->levels[matroska->num_levels - 1].length
>> + !=
>> EBML_UNKNOWN_LENGTH) {
>> + av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry
>> 0x%"PRIX32"\n", id);
>
> Would this mean a Segment with unknown length would produce this log ?
> Or Segments are not part of the levels at all ? If so, same question
> with level 1 elements which have an unknown length.
>
The part of the code you are refering to is only executed if the read
id is not part of the syntax list that is currently being used for
parsing (and if it is not one of the global elements (void or crc32)).
After the EBML head has been parsed, the segment is parsed using the
matroska_segments syntax list which of course contains an entry for
the segment. So this segment won't trigger this log no matter whether
it is unknown length or not (the segment's length hasn't been parsed
at this point btw).
But if you have an EBML Stream (a concatenation of multiple EBML
(here: Matroska) documents), then the second EBML header and the
second segment will be treated as unknown elements and it will be
acted accordingly, i.e.:
If the current level is unknown-sized and > 1, then the level will be
ended.
If the level is 1 and unknown-sized, then this will be treated as
error (we'll eventually end up in matroska_resync which will look for
level 1 elements and probably end up finding level 1 elements of the
second EBML document, but they will be treated as if the header of the
first EBML document were active. This is likely not good, but it is a
known restriction: Multiple EBML-documents are simply not supported.)
If the current level is zero or known-sized, then this level log
message will be emitted (depending on the loglevel) and it is
attempted to skip the new element. If the current-level is known-sized
and the new element unknown-sized, then according to the current
patchset, an error will be emitted; according to your proposal, the
new element will be truncated to end at the end of the current level.
The answer to the question regarding clusters is essentially the same.
And this can only happen if some error occured, namely if a cluster is
encountered at a place where no cluster is expected (e.g. if because
of a transmission error a part of a known-sized cluster is missing, so
that the next cluster is (judging by the length field of the preceding
cluster) part of the preceding cluster. This will then be treated as
"unknown entry".
(The non-incremental parsing of clusters has a similar drawback; the
current incremental parsing of clusters (which uses a syntax list
which mixes elements from different levels (which is the reason for
the error message that is the reason why I turned my attention to the
Matroska demuxer in the first place)) doesn't have this error.)
Notice that
[FFmpeg-devel] [PATCH] avcodec/diracdec: Use 64bit in intermediate of global motion vector field generation
It seems the specification does not limit the value to 32bit Fixes: signed integer overflow: -109611143 * 24 cannot be represented in type 'int' Fixes: 13477/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5648337460527104 Signed-off-by: Michael Niedermayer --- libavcodec/diracdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c index a1e759f656..a5bb6d5f34 100644 --- a/libavcodec/diracdec.c +++ b/libavcodec/diracdec.c @@ -1432,7 +1432,7 @@ static void global_mv(DiracContext *s, DiracBlock *block, int x, int y, int ref) int *b = s->globalmc[ref].pan_tilt; int *c = s->globalmc[ref].perspective; -int m = (1
Re: [FFmpeg-devel] [PATCH] avfilter/af_asetnsamples: fix sample queuing.
On Sat, Apr 06, 2019 at 09:47:07AM +0200, Paul B Mahol wrote:
> On 4/6/19, Nikolas Bowe via ffmpeg-devel wrote:
> > When asetnsamples uses output samples < input samples, remaining samples
> > build up in the fifo over time.
> > Fix this by marking the filter as ready again if there are enough samples.
> >
> > Regression since ef3babb2c70f564dc1634b3f29c6e35a2b2dc239
> > ---
> > libavfilter/af_asetnsamples.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavfilter/af_asetnsamples.c b/libavfilter/af_asetnsamples.c
> > index c60ce3063f..cab02d56f6 100644
> > --- a/libavfilter/af_asetnsamples.c
> > +++ b/libavfilter/af_asetnsamples.c
> > @@ -67,8 +67,12 @@ static int activate(AVFilterContext *ctx)
> > return ret;
> >
> > if (ret > 0) {
> > -if ((!s->pad || (s->pad && frame->nb_samples ==
> > s->nb_out_samples)))
> > -return ff_filter_frame(outlink, frame);
> > +if ((!s->pad || (s->pad && frame->nb_samples ==
> > s->nb_out_samples))) {
> > +ret = ff_filter_frame(outlink, frame);
> > +if (ff_framequeue_queued_samples(inlink) >= s->nb_out_samples)
> > +ff_filter_set_ready(ctx, 100);
> > +return ret;
> > +}
> >
> > pad_frame = ff_get_audio_buffer(outlink, s->nb_out_samples);
> > if (!pad_frame) {
> > --
> > 2.21.0.392.gf8f6787159e-goog
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
> ok
will apply
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on long files
On 4/7/2019 4:47 AM, Allan Cady via ffmpeg-devel wrote:
> [Second try submitting to the list. This patch now passes fate.]
>
> When the silencedetect filter is run against long files, the output
> timestamps gradually lose precision as the scan proceeds further into
> the file. This is because the output is formatted (in
> libavutil/timestamp.h) as "%.6g", which limits the total field
> length. Eventually, for offsets greater than 10 seconds (about 28
> hours), fractions of a second disappear altogether, and the
> timestamps are logged as whole seconds. This is insufficient
> precision for my purposes. I propose changing the format to "%.6f",
> which will give microsecond precision (probably overkill but safe)
> for all timestamps regardless of offset.
>
> The timestamp string length is limited to 32 characters
> (AV_TS_MAX_STRING_SIZE), so this should still be plenty long enough
> with the increased length (up to 10^25 seconds).
>
> My interest is in fixing this problem for silencedetect, which
> formats the timestamps by calling the macro av_ts2timestr, defined in
> timestamp.h. Since av_ts2timestr is also used in many other places (I
> count 21 c files), I have created a new macro, av_ts2timestr_format,
> with a format string added as a parameter, and left the original
> macro interface as is for other usages, to limit the scope of this
> change. The same or similar change could be made for other cases
> where better precision is desired.
>
> 0001-libavutil-timestamp.h-Fix-loss-of-precision-in-times.patch
>
> From 5492506534bf863cbf1ee8f09a5e59b4ee111226 Mon Sep 17 00:00:00 2001
> From: Allan Cady
> Date: Sun, 7 Apr 2019 00:07:58 -0700
> Subject: [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps
> for silencedetect on long files
>
> When the silencedetect filter is run against long files, the output
> timestamps gradually lose precision as the scan proceeds further into
> the file. This is because the output is formatted (in
> libavutil/timestamp.h) as "%.6g", which limits the total field
> length. Eventually, for offsets greater than 10 seconds (about 28
> hours), fractions of a second disappear altogether, and the
> timestamps are logged as whole seconds. This is insufficient
> precision for my purposes. I propose changing the format to "%.6f",
> which will give microsecond precision (probably overkill but safe)
> for all timestamps regardless of offset.
>
> The timestamp string length is limited to 32 characters
> (AV_TS_MAX_STRING_SIZE), so this should still be plenty long enough
> with the increased length (up to 10^25 seconds).
>
> My interest is in fixing this problem for silencedetect, which
> formats the timestamps by calling the macro av_ts2timestr, defined in
> timestamp.h. Since av_ts2timestr is also used in many other places (I
> count 21 c files), I have created a new macro, av_ts2timestr_format,
> with a format string added as a parameter, and left the original
> macro interface as is for other usages, to limit the scope of this
> change. The same or similar change could be made for other cases
> where better precision is desired.
> ---
> libavfilter/af_silencedetect.c | 14 --
> libavutil/timestamp.h| 13 -
> tests/ref/fate/filter-metadata-silencedetect | 2 +-
> 3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/libavfilter/af_silencedetect.c b/libavfilter/af_silencedetect.c
> index 3a71f39..2da8dbe 100644
> --- a/libavfilter/af_silencedetect.c
> +++ b/libavfilter/af_silencedetect.c
> @@ -32,6 +32,8 @@
> #include "avfilter.h"
> #include "internal.h"
>
> +const char TIMESTAMP_FMT_SILENCEDETECT[] = "%.6f";
> +
> typedef struct SilenceDetectContext {
> const AVClass *class;
> double noise; ///< noise amplitude ratio
> @@ -86,11 +88,11 @@ static av_always_inline void update(SilenceDetectContext
> *s, AVFrame *insamples,
> s->start[channel] = insamples->pts +
> av_rescale_q(current_sample / s->channels + 1 - nb_samples_notify *
> s->independent_channels / s->channels,
> (AVRational){ 1, s->last_sample_rate }, time_base);
> set_meta(insamples, s->mono ? channel + 1 : 0,
> "silence_start",
> -av_ts2timestr(s->start[channel], _base));
> +av_ts2timestr_fmt(s->start[channel], _base,
> TIMESTAMP_FMT_SILENCEDETECT));
> if (s->mono)
> av_log(s, AV_LOG_INFO, "channel: %d | ", channel);
> av_log(s, AV_LOG_INFO, "silence_start: %s\n",
> -av_ts2timestr(s->start[channel], _base));
> +av_ts2timestr_fmt(s->start[channel], _base,
> TIMESTAMP_FMT_SILENCEDETECT));
> }
> }
> } else {
> @@ -101,15 +103,15 @@ static av_always_inline void
> update(SilenceDetectContext *s, AVFrame *insamples,
> int64_t duration_ts =
Re: [FFmpeg-devel] [PATCH 14/21] avformat/matroskadec: Use proper levels after discontínuity
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> The earlier code set the level to zero upon seeking and after a
>> discontinuity although in both cases parsing (re)starts at a level 1
>> element.
>>
>> Also set the segment's length to unkown if an error occured in order
>> not
>> to drop any valid data that happens to be beyond the designated end of
>> the segment.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 59
>> +++
>> 1 file changed, 35 insertions(+), 24 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 0179e5426e..42f1c21042 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -737,13 +737,24 @@ static const char *const matroska_doctypes[] =
>> { "matroska", "webm" };
>> static int matroska_read_close(AVFormatContext *s);
>> +static int matroska_reset_status(MatroskaDemuxContext *matroska,
>> + uint32_t id, int64_t position)
>> +{
>> + matroska->current_id = id;
>> + matroska->num_levels = 1;
>> + matroska->current_cluster.pos = 0;
>> +
>> + if (position >= 0)
>> + return avio_seek(matroska->ctx->pb, position, SEEK_SET);
>> +
>> + return 0;
>> +}
>> +
>
> I think you should have done this commit in 2 parts.
> - adding matroska_reset_status() and do exactly what was done before
> - add changes related to the level and unknown length/discontinuity.
>
Ok, that's easily possible.
>> static int matroska_resync(MatroskaDemuxContext *matroska, int64_t
>> last_pos)
>> {
>> AVIOContext *pb = matroska->ctx->pb;
>> int64_t ret;
>> uint32_t id;
>> - matroska->current_id = 0;
>> - matroska->num_levels = 0;
>> /* seek to next position to resync from */
>> if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) {
>> @@ -759,7 +770,14 @@ static int matroska_resync(MatroskaDemuxContext
>> *matroska, int64_t last_pos)
>> id == MATROSKA_ID_CUES || id ==
>> MATROSKA_ID_TAGS ||
>> id == MATROSKA_ID_SEEKHEAD || id ==
>> MATROSKA_ID_ATTACHMENTS ||
>> id == MATROSKA_ID_CLUSTER || id ==
>> MATROSKA_ID_CHAPTERS) {
>> - matroska->current_id = id;
>> + /* Prepare the context for further parsing of a level 1
>> element. */
>> + matroska_reset_status(matroska, id, -1);
>
> You set num_levels to 1 now, leaving this function used to have
> num_levels set to 0. I'm not sure it's correct or not.
>
We have found a level 1 element, so the level should be set to 1. If
we enter and parse the level 1 element found, we will be at level two;
currently we are only at level 1 (so that the level 1 element appears
to be the highest level in the hierarchy, when in fact it is not). So
this change is intentional.
>> +
>> + /* Given that we are here means that an error has occured,
>
> I thought this function was meant to find a level1 ID after getting
> into a Segment. This does not seem like an error at all.
>
First the EBML header is parsed as a typical EBML_NEST element; then
the segment is parsed as nest, too. The syntax according to which its
children are parsed contains all level 1 elements; except for a
cluster (which is an EBML_STOP element) all elements are EBML_LEVEL1
and are therefore parsed according to the respective syntax elements
for their descendants. For normal files, there is no need to resync at
all: The next element begins directly after the previous one has ended.
matroska_resync is only used in two situations:
If parsing the segment doesn't find a cluster and we are not in the
mode for parsing live header files. (This implies that an error
(possibly EOF) has occurred.)
And if an error happened during reading a packet (i.e. in
matroska_parse_cluster()).
There is actually no reason to resync for valid files at all. (When
reading till the end, current FFmpeg would use matroska_resync() at
the end even if everything actually looks fine. Patch 20 is designed
to fix this.)
>> + * so treat the segment as unknown length in order not to
>> + * discard valid data that happens to be beyond the
>> designated
>> + * end of the segment. */
>> + matroska->levels[0].length = EBML_UNKNOWN_LENGTH;
>> return 0;
>> }
>> id = (id << 8) | avio_r8(pb);
>> @@ -1610,18 +1628,12 @@ static int
>> matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
>> matroska->current_id = 0;
>> ret = ebml_parse(matroska, matroska_segment, matroska);
>> -
>> - /* remove dummy level */
>> - while (matroska->num_levels) {
>> - uint64_t length =
>> matroska->levels[--matroska->num_levels].length;
>> - if (length == EBML_UNKNOWN_LENGTH)
>> - break;
>> - }
>
> I think
Re: [FFmpeg-devel] [PATCH 13/21] avformat/matroskadec: Improve length check
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> The earlier code had three flaws:
>>
>> 1. The case of an unknown-sized element inside a finite-sized element
>> (which is against the specifications) was not caught.
>>
>> 2. The error message wasn't helpful: It compared the length of the
>> child
>> with the offset of the end of the parent and claimed that the first
>> exceeds the latter, although that is not necessarily true.
>>
>> 3. Unknown-sized elements that are not parsed can't be skipped. Given
>> that according to the Matroska specifications only the segment and the
>> clusters can be of unknown-size, this is handled by not allowing any
>
> This restriction is rather new. There might be old files that use
> infinite sizes in other places.
>
Indeed there are. The file mentioned by Dale
(https://ffmpeg.org/pipermail/ffmpeg-devel/2019-February/240376.html;
https://cs.chromium.org/chromium/src/media/test/data/bear-320x240-live.webm)
has cues where every master element (even CueTrackPositions) are
written as unknown-sized element (with length fields coded on eight
bytes). The Cues aren't referenced in a Seekhead at the beginning
though, so that FFmpeg doesn't try to read them. If they were
referenced, neither current nor patched FFmpeg would be able to
properly parse them and patched FFmpeg would furthermore give an error
message.
When exactly has this restriction been enacted?
I'll see if I find a way to support such elements (although I am not
sure whether it is worthwhile).
>> other units to have infinite size whereas the earlier code would seek
>> back by 1 byte upon encountering an infinite-size element that ought
>> to be skipped.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 26 ++
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 6fa324c0cc..0179e5426e 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1180,11 +1180,29 @@ static int
>> ebml_parse_elem(MatroskaDemuxContext *matroska,
>> if (matroska->num_levels > 0) {
>> MatroskaLevel *level =
>> >levels[matroska->num_levels - 1];
>> int64_t pos = avio_tell(pb);
>> - if (level->length != EBML_UNKNOWN_LENGTH &&
>> - (pos + length) > (level->start + level->length)) {
>> +
>> + if (length != EBML_UNKNOWN_LENGTH &&
>> + level->length != EBML_UNKNOWN_LENGTH) {
>> + uint64_t elem_end = pos + length,
>> + level_end = level->start + level->length;
>> +
>> + if (level_end < elem_end) {
>> + av_log(matroska->ctx, AV_LOG_ERROR,
>> + "Element at 0x%"PRIx64" ending at
>> 0x%"PRIx64" exceeds "
>> + "containing master element ending at
>> 0x%"PRIx64"\n",
>> + pos, elem_end, level_end);
>> + return AVERROR_INVALIDDATA;
>> + }
>> + } else if (level->length != EBML_UNKNOWN_LENGTH) {
>> + av_log(matroska->ctx, AV_LOG_ERROR, "Unknown-sized
>> element "
>> + "at 0x%"PRIx64" inside parent with finite
>> size\n", pos);
>> + return AVERROR_INVALIDDATA;
>
> IMO there's no reason to return an error here. You can log the error
> and still parse the data, it should end up fine (if the code is done
> as such that you can never read past your parents boundaries). At
> least libebml can deal with that.
>
Ok, treating such elements as if they extended to the end of their
master element is easily doable.
>> + } else if (length == EBML_UNKNOWN_LENGTH && id !=
>> MATROSKA_ID_CLUSTER) {
>> + // According to the specifications only clusters
>> and segments
>> + // are allowed to be unknown-sized.
>> av_log(matroska->ctx, AV_LOG_ERROR,
>> - "Invalid length 0x%"PRIx64" > 0x%"PRIx64" in
>> parent\n",
>> - length, level->start + level->length);
>> + "Found unknown-sized element other than a
>> cluster at "
>> + "0x%"PRIx64". Dropping the invalid
>> element.\n", pos);
>> return AVERROR_INVALIDDATA;
>> }
>> }
>> --
>> 2.19.2
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avcodec: add bink2 video decoder
On Sun, Apr 07, 2019 at 10:00:09AM +0200, Paul B Mahol wrote:
> On 4/7/19, Peter Ross wrote:
> > On Wed, Mar 27, 2019 at 09:21:47PM +0100, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol
> >> ---
> >> Missing deblocking.
> >> ---
> >> configure |1 +
> >> libavcodec/Makefile |1 +
> >> libavcodec/allcodecs.c |1 +
> >> libavcodec/avcodec.h|1 +
> >> libavcodec/bink2.c | 787 +++
> >> libavcodec/bink2f.c | 1139 +
> >> libavcodec/bink2g.c | 1342 +++
> >> +static int bink2g_get_type(GetBitContext *gb, int *lru)
> >> +{
> >> +int val;
> >> +
> >> +ff_dlog(NULL, " type show %X @ %d\n", show_bits(gb, 4),
> >> get_bits_count(gb));
> >> +switch (get_unary(gb, 1, 3)) {
> >> +case 0:
> >> +val = lru[0];
> >> +break;
> >> +case 1:
> >> +val = lru[1];
> >> +FFSWAP(int, lru[0], lru[1]);
> >> +break;
> >> +case 2:
> >> +val = lru[3];
> >> +FFSWAP(int, lru[2], lru[3]);
> >> +break;
> >> +case 3:
> >> +val = lru[2];
> >> +FFSWAP(int, lru[1], lru[2]);
> >> +break;
> >> +}
> >
> > cases 1-3 could be collapsed.
>
> I do not see how.
reorder the second and third elements in bink2g_decode_slice types_lru[].
that at least removes the val = lru[x] statement from each switch case.
as for the swaps, there is some (probably unintentional) symetry going on there.
the result could be:
int n = get_unary(gb, 1, 3)
int v = lru[n];
if (n) {
int n2 = n == 1 ? 1 : 3;
FFSWAP(int, lru[n - 1], lru[n2]);
}
return v;
this is a minor cleanup and in the scheme of things, may not even be worth it.
-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 10/21] avformat/matroskadec: Don't keep old blocks
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> Before this commit, the Matroska muxer would read a block when required
>> to do so, parse the block, create and return the necessary AVPackets
>> and
>> yet keep the blocks (in a dynamically allocated list), although they
>> aren't used at all any more. This has been changed. There is no list
>> any
>> more and the block is immediately discarded after parsing.
>
> My understanding of the code is that the blocks are put in a queue,
Yes, the parsed blocks are put in a queue of their own; so we don't
need to keep all the raw data of all the already parsed blocks of the
current cluster around. (Refcounting means that some of this data
might be keep a little longer though, but even in this case this patch
eliminates e.g. the constant reallocation of the list of blocks.)
> that's whatwebm_clusters_start_with_keyframe() uses to check that the
> first frame is a keyframe
Yes.
> (it doesn't check the type of the frame though...).
I see a "if (!(pkt->flags & AV_PKT_FLAG_KEY))" in there.
> But since there's only one read
> inmatroska_parse_cluster_incremental()there's only one kept in memory.
>
> So LGTM.
>
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 87
>> +--
>> 1 file changed, 38 insertions(+), 49 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 9198fa1bea..997c96d78f 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -304,9 +304,20 @@ typedef struct MatroskaLevel {
>> uint64_t length;
>> } MatroskaLevel;
>> +typedef struct MatroskaBlock {
>> + uint64_t duration;
>> + int64_t reference;
>> + uint64_t non_simple;
>> + EbmlBin bin;
>> + uint64_t additional_id;
>> + EbmlBin additional;
>> + int64_t discard_padding;
>> +} MatroskaBlock;
>> +
>> typedef struct MatroskaCluster {
>> + MatroskaBlock block;
>> uint64_t timecode;
>> - EbmlList blocks;
>> + int64_t pos;
>> } MatroskaCluster;
>> typedef struct MatroskaLevel1Element {
>> @@ -356,8 +367,6 @@ typedef struct MatroskaDemuxContext {
>> MatroskaLevel1Element level1_elems[64];
>> int num_level1_elems;
>> - int current_cluster_num_blocks;
>> - int64_t current_cluster_pos;
>> MatroskaCluster current_cluster;
>> /* WebM DASH Manifest live flag */
>> @@ -367,16 +376,6 @@ typedef struct MatroskaDemuxContext {
>> int bandwidth;
>> } MatroskaDemuxContext;
>> -typedef struct MatroskaBlock {
>> - uint64_t duration;
>> - int64_t reference;
>> - uint64_t non_simple;
>> - EbmlBin bin;
>> - uint64_t additional_id;
>> - EbmlBin additional;
>> - int64_t discard_padding;
>> -} MatroskaBlock;
>> -
>> static const EbmlSyntax ebml_header[] = {
>> { EBML_ID_EBMLREADVERSION, EBML_UINT, 0, offsetof(Ebml,
>> version), { .u = EBML_VERSION } },
>> { EBML_ID_EBMLMAXSIZELENGTH, EBML_UINT, 0, offsetof(Ebml,
>> max_size), { .u = 8 } },
>> @@ -705,9 +704,9 @@ static const EbmlSyntax matroska_blockgroup[] = {
>> };
>> static const EbmlSyntax matroska_cluster_parsing[] = {
>> - { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT,
>> 0, offsetof(MatroskaCluster, timecode) },
>> - { MATROSKA_ID_BLOCKGROUP, EBML_NEST,
>> sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n =
>> matroska_blockgroup } },
>> - { MATROSKA_ID_SIMPLEBLOCK, EBML_PASS,
>> sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n =
>> matroska_blockgroup } },
>> + { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,
>> offsetof(MatroskaCluster, timecode) },
>> + { MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n =
>> matroska_blockgroup } },
>> + { MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, 0, 0, { .n =
>> matroska_blockgroup } },
>> { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
>> { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
>> { MATROSKA_ID_INFO, EBML_NONE },
>> @@ -3443,57 +3442,48 @@ end:
>> static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
>> {
>> - EbmlList *blocks_list;
>> - MatroskaBlock *blocks;
>> - int i, res;
>> + MatroskaCluster *cluster = >current_cluster;
>> + MatroskaBlock *block = >block;
>> + int res;
>> res = ebml_parse(matroska,
>> matroska_cluster_parsing,
>> - >current_cluster);
>> + cluster);
>> if (res == 1) {
>> /* New Cluster */
>> - if (matroska->current_cluster_pos)
>> + if (cluster->pos)
>> ebml_level_end(matroska);
>> - ebml_free(matroska_cluster_parsing,
>> >current_cluster);
>> - memset(>current_cluster, 0,
>> sizeof(MatroskaCluster));
>> - matroska->current_cluster_num_blocks = 0;
>> - matroska->current_cluster_pos =
>>
Re: [FFmpeg-devel] [PATCH 08/21] avformat/matroskadec: Improve error messages
Steve Lhomme:
> On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
>> ebml_read_num had a number of flaws:
>>
>> 1. The check for read errors/EOF was totally wrong. E.g. an EBML number
>> beginning with the invalid 0x00 would be considered a read error,
>> although it is just invalid data.
>> 2. The check for read errors/EOF was done just once, after reading the
>> first byte of the EBML number. But errors/EOF can happen inbetween, of
>> course, and this wasn't checked.
>> 3. There was no way to distinguish when EOF should be an error (because
>> the data has to be there) for which an error message should be emitted
>> and when it is not necessarily an error (namely during parsing of EBML
>> IDs). Such a possibility has been added and used.
>
> Maybe the title of the patch should rather mention that it's fixing
> the EOF handling when reading EBML ID/Length. The changed error
> messages is a less important consequence.
>
How about "avformat/matroskadec: Improve (in particular) EOF checks"?
>
>>
>> Some useless initializations were also fixed.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/matroskadec.c | 61
>> ++-
>> 1 file changed, 34 insertions(+), 27 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index a6617a607b..aa2266384a 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -820,29 +820,19 @@ static int ebml_level_end(MatroskaDemuxContext
>> *matroska)
>> * Returns: number of bytes read, < 0 on error
>> */
>> static int ebml_read_num(MatroskaDemuxContext *matroska,
>> AVIOContext *pb,
>> - int max_size, uint64_t *number)
>> + int max_size, uint64_t *number, int
>> eof_forbidden)
>> {
>> - int read = 1, n = 1;
>> - uint64_t total = 0;
>> + int read, n = 1;
>> + uint64_t total;
>> + int64_t pos;
>> - /* The first byte tells us the length in bytes - avio_r8()
>> can normally
>> - * return 0, but since that's not a valid first ebmlID byte, we
>> can
>> - * use it safely here to catch EOS. */
>> - if (!(total = avio_r8(pb))) {
>> - /* we might encounter EOS here */
>> - if (!avio_feof(pb)) {
>> - int64_t pos = avio_tell(pb);
>> - av_log(matroska->ctx, AV_LOG_ERROR,
>> - "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
>> - pos, pos);
>> - return pb->error ? pb->error : AVERROR(EIO);
>> - }
>> - return AVERROR_EOF;
>> - }
>> + /* The first byte tells us the length in bytes - except when it
>> is zero. */
>> + total = avio_r8(pb);
>> + if (avio_feof(pb))
>> + goto err;
>> /* get the length of the EBML number */
>> - read = 8 - ff_log2_tab[total];
>> - if (read > max_size) {
>> + if (!total || (read = 8 - ff_log2_tab[total]) > max_size) {
>> int64_t pos = avio_tell(pb) - 1;
>> av_log(matroska->ctx, AV_LOG_ERROR,
>> "Invalid EBML number size tag 0x%02x at pos
>> %"PRIu64" (0x%"PRIx64")\n",
>> @@ -855,9 +845,27 @@ static int ebml_read_num(MatroskaDemuxContext
>> *matroska, AVIOContext *pb,
>> while (n++ < read)
>> total = (total << 8) | avio_r8(pb);
>> + if (avio_feof(pb)) {
>> + eof_forbidden = 1;
>> + goto err;
>> + }
>
> You're forcing an error if the data ends after reading a number ?
> Ending a Matroska file with a number should be fine. It could also be
> an element with a size of 0. It doesn't contain any data but it's
> still valid (depending on the semantic of the element).
>
> So this forced error seem wrong. Let the next read catch the EOF if it
> finds one.
>
avio_feof doesn't indicate an error if we reach EOF, only if we
attempt to read past EOF (or if another error occurred). If the EBML
number we intend to parse is completely read, then no error is
indicated here at all. If the input ends immediately after this EBML
number, then the next read will trigger EOF and will have to catch it.
If avio_feof is set at this point, it means that the first byte of the
EBML number says that the EBML number is total bytes long, but an
error or EOF occured during reading of these bytes. I think this
warrants an error. eof_forbidden is set at this point so that an
incomplete EBML number will always trigger an error.
>> +
>> *number = total;
>> return read;
>> +
>> +err:
>> + pos = avio_tell(pb);
>> + if (pb->error) {
>> + av_log(matroska->ctx, AV_LOG_ERROR,
>> + "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
>> + pos, pos);
>> + return pb->error;
>> + }
>> + if (eof_forbidden)
>> + av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
>> + "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
>
> If eof_forbidden is false (EOF allowed) you return an EOF error anyway ?
>
Yes, so
Re: [FFmpeg-devel] [PATCH 05/21] avformat/matroskadec: Set offset of first cluster
Steve Lhomme: > On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote: >> By default, the data_offset member of the AVFormatInternal of the >> AVFormatContext associated with the MatroskaDemuxContext has not been >> initialized explicitly by any Matroska-specific function, so that it >> was >> initialized by default to the offset at the end of >> matroska_read_header, >> i.e. usually to the offset of the length field of the first encountered >> cluster. This meant that in case that the Matroska-specific seek-code >> fails because there are no index entries for the target track a seek to >> data_offset would be performed and ordinary parsing would start from >> there which is nonsense: The length field would be treated as EBML >> ID and >> (if the length field is not longer than four bytes (EBML numbers that >> long are rejected as invalid EBML IDs)) and whatever comes next >> would be >> treated as its EBML size although it simply isn't. >> >> Signed-off-by: Andreas Rheinhardt >> --- >> libavformat/matroskadec.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >> index 49f8ff4082..f9811b54a1 100644 >> --- a/libavformat/matroskadec.c >> +++ b/libavformat/matroskadec.c >> @@ -2651,6 +2651,9 @@ static int >> matroska_read_header(AVFormatContext *s) >> pos = avio_tell(matroska->ctx->pb); >> res = ebml_parse(matroska, matroska_segment, matroska); >> } >> + /* Set data_offset as it might be needed later by >> seek_frame_generic. */ >> + if (matroska->current_id) > > I'm surprised this doesn't error out if a (level 1) ID is not found here. >There are two ways how ebml_parse can return 1 in the earlier call: It can find (and consume) a cluster ID; or it can hit EOF if it is in the mode for parsing live header files (these don't contain clusters at all). In the latter case, matroska->current_id is unset. And if we are in the normal (non-live-header) mode and no cluster has been found, then we will end up at fail anyway (when matroska_resync eventually hits the end of input). Thinking about this, there is something wrong with this approach: A Matroska file needn't contain a cluster at all (e.g. a chapter- or tags- or attachment-only file is not against the spec if I am not mistaken). But this is orthogonal to the patch at hand. >> + s->internal->data_offset = avio_tell(matroska->ctx->pb) - 4; > > The "- 4" is OK as long as level 1 elements are always 4 bytes (which > is the case). But if matroska_resync() ever exits if it finds an EBML > Void or CRC-32 then this will break. > > The code is safe for now but may not be future proof. > As has already been said: If matroska->current_id is set at this point, then it contains a cluster ID, not an arbitrary ID, because the IDs that matroska_resync finds are fed to ebml_parse before this check and a level 1 ID different than a cluster is parsed. Nevertheless, the check should probably be changed to explicitly check for a cluster ID, just to make this more readable. >> matroska_execute_seekhead(matroska); >> if (!matroska->time_scale) >> -- >> 2.19.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 19/21] avformat/matroskadec: Add SilentTracks to cluster syntax
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
This is important as unknown-sized elements end upon encountering an
unknown EBML ID.
That's the problem with this approach. Noone is allowed to use their own
custom tags (RAWCooked for example) and the unknown length feature. You
have to know every element in the world for this not to break.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroska.h| 1 +
libavformat/matroskadec.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 86968a8de1..43fea595b6 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -224,6 +224,7 @@
#define MATROSKA_ID_CLUSTERTIMECODE 0xE7
#define MATROSKA_ID_CLUSTERPOSITION 0xA7
#define MATROSKA_ID_CLUSTERPREVSIZE 0xAB
+#define MATROSKA_ID_SILENTTRACKS 0x5854
#define MATROSKA_ID_BLOCKGROUP 0xA0
#define MATROSKA_ID_BLOCKADDITIONS 0x75A1
#define MATROSKA_ID_BLOCKMORE 0xA6
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 60f58cefa9..c1feb3f0a1 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -711,6 +711,7 @@ static const EbmlSyntax matroska_cluster_parsing[] = {
{ MATROSKA_ID_BLOCKGROUP, EBML_STOP },
{ MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
{ MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
+{ MATROSKA_ID_SILENTTRACKS,EBML_NONE },
{ 0 }
};
--
2.19.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avcodec: add bink2 video decoder
On 4/7/19, Peter Ross wrote:
> On Wed, Mar 27, 2019 at 09:21:47PM +0100, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol
>> ---
>> Missing deblocking.
>> ---
>> configure |1 +
>> libavcodec/Makefile |1 +
>> libavcodec/allcodecs.c |1 +
>> libavcodec/avcodec.h|1 +
>> libavcodec/bink2.c | 787 +++
>> libavcodec/bink2f.c | 1139 +
>> libavcodec/bink2g.c | 1342 +++
>> libavcodec/codec_desc.c |7 +
>> libavformat/bink.c |3 +-
>
> my comments below.
>
> this is a mammoth amount of work deserving of a better quality review.
>
>> +++ b/libavcodec/bink2.c
>> @@ -0,0 +1,787 @@
>> +/*
>> + * Bink video 2 decoder
>> + * Copyright (c) 2014 Konstantin Shishkov
>> + * Copyright (c) 2019 Paul B Mahol
>> + *
>
>> +static const uint8_t luma_repos[] = {
>> +0, 1, 4, 5, 2, 3, 6, 7, 8, 9, 12, 13, 10, 11, 14, 15,
>> +};
>
> identical to msvideo1enc.c remap
> consider moving to libavcodec/mathops.h
>
>> +static const int32_t bink2g_dc_pat[] = {
>> +1024, 1218, 1448, 1722, 2048,
>> +2435, 2896, 3444, 4096, 4871,
>> +5793, 6889, 8192, 9742, 11585, 13777, 16384,
>> +19484, 23170, 27555, 32768, 38968, 46341,
>> +55109, 65536, 77936, 92682, 110218, 131072,
>> +155872, 185364, 220436, 262144, 311744,
>> +370728, 440872, 524288,
>> +};
>
> this is so close to ff_dirac_qscale_tab it is not funny.
>
>> + 3, 24, 11, 18, 25, 13, 14, 4,
>> +15, 5, 6, 7, 12, 19, 20, 21,
>> +22, 23, 26, 27, 28, 29, 30, 31,
>> +32, 33, 34, 35, 36, 37, 38, 39,
>> +40, 41, 42, 43, 44, 45, 46, 47,
>> +48, 49, 50, 51, 52, 53, 54, 55,
>> +56, 57, 58, 59, 60, 61, 62, 63
>> +};
>> +
>> +static const uint8_t bink2g_scan[64] = {
>> + 0, 8, 1, 2, 9, 16, 24, 17,
>> +10, 3, 4, 11, 18, 25, 32, 40,
>> +33, 26, 19, 12, 5, 6, 13, 20,
>> +27, 34, 41, 48, 56, 49, 42, 35,
>> +28, 21, 14, 7, 15, 22, 29, 36,
>> +43, 50, 57, 58, 51, 44, 37, 30,
>> +23, 31, 38, 45, 52, 59, 60, 53,
>> +46, 39, 47, 54, 61, 62, 55, 63,
>> +};
>
> this table has more white space than the other scan tables...
>
>> +enum BlockTypes {
>> +INTRA_BLOCK = 0, ///< intra DCT block
>> +SKIP_BLOCK, ///< skipped block
>> +MOTION_BLOCK,///< block is copied from previous frame with some
>> offset
>> +RESIDUE_BLOCK, ///< motion block with some difference added
>> +};
>> +
>> +static const uint8_t ones_count[16] = {
>> +0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4
>> +};
>
> duplicate of libavcodec/vc1_mc.c popcount4
>
>> +static int bink2_decode_frame(AVCodecContext *avctx, void *data,
>> + int *got_frame, AVPacket *pkt)
>> +{
>> +Bink2Context * const c = avctx->priv_data;
>> +GetBitContext *gb = >gb;
>> +AVFrame *frame = data;
>> +uint8_t *dst[4];
>> +uint8_t *src[4];
>> +int stride[4];
>> +int sstride[4];
>> +uint32_t off = 0;
>> +int is_kf = !!(pkt->flags & AV_PKT_FLAG_KEY);
>> +int ret, w, h;
>> +int height_a;
>> +
>> +w = avctx->width;
>> +h = avctx->height;
>> +ret = ff_set_dimensions(avctx, FFALIGN(w, 32), FFALIGN(h, 32));
>> +if (ret < 0)
>> +return ret;
>> +avctx->width = w;
>> +avctx->height = h;
>> +
>> +if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
>> +return ret;
>
> ret error handling style is inconsistent
>
>> +static int bink2f_decode_inter_luma(Bink2Context *c,
>> +float block[4][64],
>> +unsigned *prev_cbp, int *prev_q,
>> +uint8_t *dst, int stride,
>> +int flags)
>> +{
>> +GetBitContext *gb = >gb;
>> +float *dc = c->current_dc[c->mb_pos].dc[c->comp];
>> +unsigned cbp;
>> +int q, dq;
>> +
>> +*prev_cbp = cbp = bink2f_decode_cbp_luma(gb, *prev_cbp);
>> +dq = bink2f_decode_delta_q(gb);
>> +q = *prev_q + dq;
>> +if (q < 0 || q >= 16)
>> +return AVERROR_INVALIDDATA;
>> +*prev_q = q;
>> +
>> +bink2f_decode_dc(c, gb, dc, 1, q, -1023, 1023, 0xA8);
>> +
>> +for (int i = 0; i < 4; i++) {
>> +bink2f_decode_ac(gb, bink2f_luma_scan, block, cbp >> (i * 4),
>> + bink2f_ac_quant[q], bink2f_luma_inter_qmat);
>
> check error code?
>
>> +for (int j = 0; j < 4; j++) {
>> +block[j][0] = dc[i * 4 + j] * 0.125f;
>> +bink2f_idct_add(dst + (luma_repos[i*4+j]&3) * 8 +
>> +(luma_repos[i*4+j]>>2) * 8 * stride, stride,
>> +block[j]);
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int bink2f_decode_inter_chroma(Bink2Context *c,
>> + float block[4][64],
>> +
Re: [FFmpeg-devel] [PATCH 18/21] avformat/matroskadec: Combine two arrays
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
By including SimpleBlocks and Blocksgroups twice in the same EbmlSyntax
array (with different semantics), one can reduce the duplication of the
other values.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 3adcb3e86d..60f58cefa9 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -704,25 +704,18 @@ static const EbmlSyntax matroska_blockgroup[] = {
};
static const EbmlSyntax matroska_cluster_parsing[] = {
-{ MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster,
timecode) },
-{ MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n = matroska_blockgroup
} },
{ MATROSKA_ID_SIMPLEBLOCK, EBML_BIN, 0, offsetof(MatroskaBlock, bin)
},
-{ MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
-{ MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
-{ 0 }
-};
-
-static const EbmlSyntax matroska_cluster_initial[] = {
+{ MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n = matroska_blockgroup
} },
{ MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster,
timecode) },
-{ MATROSKA_ID_BLOCKGROUP, EBML_STOP },
{ MATROSKA_ID_SIMPLEBLOCK, EBML_STOP },
+{ MATROSKA_ID_BLOCKGROUP, EBML_STOP },
{ MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
{ MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
{ 0 }
};
static const EbmlSyntax matroska_cluster_enter[] = {
-{ MATROSKA_ID_CLUSTER, EBML_NEST, 0, 0, { .n =
matroska_cluster_initial } },
+{ MATROSKA_ID_CLUSTER, EBML_NEST, 0, 0, { .n =
_cluster_parsing[2] } },
To avoid breaking this optimisation when the code is changed you might
use some static_assert to make sure that matroska_cluster_parsing[2].id
is MATROSKA_ID_CLUSTERTIMECODE
{ 0 }
};
--
2.19.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 15/21] avformat/matroskadec: Redo level handling
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
This commit changes how levels are handled: If the level used for
ebml_parse ends directly after an element that has been consumed, then
ebml_parse ends the level itself (and any finite-sized levels that end
there as well) and informs the caller via the return value; if the
current level is unknown-sized, then the level is ended as soon as
an element that is not valid on the current level is encountered.
This is designed for situations where one wants to parse master elements
incrementally, i.e. not in one go via ebml_parse_nest.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 104 +++---
1 file changed, 85 insertions(+), 19 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 42f1c21042..32cf57685f 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -69,6 +69,8 @@
#include "qtpalette.h"
#define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length, in uint64_t */
+#define LEVEL_ENDED 2 /* return value of ebml_parse when the
+ * syntax level used for parsing
ended. */
typedef enum {
EBML_NONE,
@@ -1041,16 +1043,32 @@ static int ebml_parse_id(MatroskaDemuxContext
*matroska, EbmlSyntax *syntax,
uint32_t id, void *data)
{
int i;
+
for (i = 0; syntax[i].id; i++)
if (id == syntax[i].id)
break;
-if (!syntax[i].id && id == MATROSKA_ID_CLUSTER &&
-matroska->num_levels > 0 &&
-matroska->levels[matroska->num_levels - 1].length ==
EBML_UNKNOWN_LENGTH)
-return 0; // we reached the end of an unknown size cluster
+
if (!syntax[i].id && id != EBML_ID_VOID && id != EBML_ID_CRC32) {
-av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry 0x%"PRIX32"\n", id);
+if (!matroska->num_levels || matroska->levels[matroska->num_levels -
1].length
+!=
EBML_UNKNOWN_LENGTH) {
+av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry 0x%"PRIX32"\n",
id);
Would this mean a Segment with unknown length would produce this log ?
Or Segments are not part of the levels at all ? If so, same question
with level 1 elements which have an unknown length.
+} else if (matroska->num_levels > 1) {
+// Unknown-sized master elements (except root elements) end
+// when an id not known to be allowed in them is encountered.
+matroska->num_levels--;
+return LEVEL_ENDED;
+} else if (matroska->num_levels == 1) {
+AVIOContext *pb = matroska->ctx->pb;
+int64_t pos = avio_tell(pb);
+// An unkown level 1 element inside an unknown-sized segment
+// is considered an error.
+av_log(matroska->ctx, AV_LOG_ERROR, "Found unknown element id
0x%"PRIX32
+ " at pos. %"PRIu64" (0x%"PRIx64") in an unknown-sized segment.
"
+ "Will be treated as error.\n", id, pos, pos);
+return AVERROR_INVALIDDATA;
+}
}
+
return ebml_parse_elem(matroska, [i], data);
}
@@ -1061,9 +1079,24 @@ static int ebml_parse(MatroskaDemuxContext *matroska, EbmlSyntax *syntax,
uint64_t id;
int res = ebml_read_num(matroska, matroska->ctx->pb, 4, , 0);
if (res < 0) {
-// in live mode, finish parsing if EOF is reached.
-return (matroska->is_live && matroska->ctx->pb->eof_reached &&
-res == AVERROR_EOF) ? 1 : res;
+if (matroska->ctx->pb->eof_reached && res == AVERROR_EOF) {
+if (matroska->is_live)
+// in live mode, finish parsing if EOF is reached.
+return 1;
+if (matroska->num_levels) {
+MatroskaLevel *level =
>levels[matroska->num_levels - 1];
+
+if (level->length == EBML_UNKNOWN_LENGTH) {
+// Unknown-sized elements automatically end at EOF.
+matroska->num_levels = 0;
+return LEVEL_ENDED;
+} else if (avio_tell(matroska->ctx->pb) < level->start +
level->length) {
+av_log(matroska->ctx, AV_LOG_ERROR, "File ended before
"
+ "its declared end\n");
+}
+}
+}
+return res;
}
matroska->current_id = id | 1 << 7 * res;
}
@@ -1098,10 +1131,15 @@ static int ebml_parse_nest(MatroskaDemuxContext
*matroska, EbmlSyntax *syntax,
break;
}
-while (!res && !ebml_level_end(matroska))
+if (!matroska->levels[matroska->num_levels - 1].length) {
+matroska->num_levels--;
+return 0;
+}
+
+
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/pnm_parser: Factor out next/index compensation
On 4/7/19, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/pnm_parser.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/pnm_parser.c b/libavcodec/pnm_parser.c
> index 91a9edc016..95241c30b3 100644
> --- a/libavcodec/pnm_parser.c
> +++ b/libavcodec/pnm_parser.c
> @@ -70,19 +70,17 @@ retry:
> c = *bs++;
> } else if (c == 'P') {
> next = bs - pnmctx.bytestream_start + skip - 1;
> -if (pnmctx.bytestream_start != buf + skip)
> -next -= pc->index;
> break;
> }
> }
> } else {
> next = pnmctx.bytestream - pnmctx.bytestream_start + skip
> + av_image_get_buffer_size(avctx->pix_fmt, avctx->width,
> avctx->height, 1);
> -if (pnmctx.bytestream_start != buf + skip)
> -next -= pc->index;
> -if (next > buf_size)
> -next = END_NOT_FOUND;
> }
> +if (next != END_NOT_FOUND && pnmctx.bytestream_start != buf + skip)
> +next -= pc->index;
> +if (next > buf_size)
> +next = END_NOT_FOUND;
>
> if (ff_combine_frame(pc, next, , _size) < 0) {
> *poutbuf = NULL;
> --
> 2.21.0
probably ok
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/pnm_parser: Factor next initialization out
On 4/7/19, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/pnm_parser.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/libavcodec/pnm_parser.c b/libavcodec/pnm_parser.c
> index e3bfa3c490..91a9edc016 100644
> --- a/libavcodec/pnm_parser.c
> +++ b/libavcodec/pnm_parser.c
> @@ -47,6 +47,7 @@ retry:
> pnmctx.bytestream = (uint8_t *) buf + skip; /* casts avoid
> warnings */
> pnmctx.bytestream_end = (uint8_t *) buf + buf_size - skip;
> }
> +next = END_NOT_FOUND;
> if (ff_pnm_decode_header(avctx, ) < 0) {
> if (pnmctx.bytestream < pnmctx.bytestream_end) {
> if (pc->index) {
> @@ -58,12 +59,10 @@ retry:
> }
> goto retry;
> }
> -next = END_NOT_FOUND;
> } else if (pnmctx.type < 4) {
>uint8_t *bs = pnmctx.bytestream;
> const uint8_t *end = pnmctx.bytestream_end;
>
> -next = END_NOT_FOUND;
> while (bs < end) {
> int c = *bs++;
> if (c == '#') {
> --
> 2.21.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on long files
[Second try submitting to the list. This patch now passes fate.]
When the silencedetect filter is run against long files, the output
timestamps gradually lose precision as the scan proceeds further into
the file. This is because the output is formatted (in
libavutil/timestamp.h) as "%.6g", which limits the total field
length. Eventually, for offsets greater than 10 seconds (about 28
hours), fractions of a second disappear altogether, and the
timestamps are logged as whole seconds. This is insufficient
precision for my purposes. I propose changing the format to "%.6f",
which will give microsecond precision (probably overkill but safe)
for all timestamps regardless of offset.
The timestamp string length is limited to 32 characters
(AV_TS_MAX_STRING_SIZE), so this should still be plenty long enough
with the increased length (up to 10^25 seconds).
My interest is in fixing this problem for silencedetect, which
formats the timestamps by calling the macro av_ts2timestr, defined in
timestamp.h. Since av_ts2timestr is also used in many other places (I
count 21 c files), I have created a new macro, av_ts2timestr_format,
with a format string added as a parameter, and left the original
macro interface as is for other usages, to limit the scope of this
change. The same or similar change could be made for other cases
where better precision is desired.
From 5492506534bf863cbf1ee8f09a5e59b4ee111226 Mon Sep 17 00:00:00 2001
From: Allan Cady
Date: Sun, 7 Apr 2019 00:07:58 -0700
Subject: [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps
for silencedetect on long files
When the silencedetect filter is run against long files, the output
timestamps gradually lose precision as the scan proceeds further into
the file. This is because the output is formatted (in
libavutil/timestamp.h) as "%.6g", which limits the total field
length. Eventually, for offsets greater than 10 seconds (about 28
hours), fractions of a second disappear altogether, and the
timestamps are logged as whole seconds. This is insufficient
precision for my purposes. I propose changing the format to "%.6f",
which will give microsecond precision (probably overkill but safe)
for all timestamps regardless of offset.
The timestamp string length is limited to 32 characters
(AV_TS_MAX_STRING_SIZE), so this should still be plenty long enough
with the increased length (up to 10^25 seconds).
My interest is in fixing this problem for silencedetect, which
formats the timestamps by calling the macro av_ts2timestr, defined in
timestamp.h. Since av_ts2timestr is also used in many other places (I
count 21 c files), I have created a new macro, av_ts2timestr_format,
with a format string added as a parameter, and left the original
macro interface as is for other usages, to limit the scope of this
change. The same or similar change could be made for other cases
where better precision is desired.
---
libavfilter/af_silencedetect.c | 14 --
libavutil/timestamp.h| 13 -
tests/ref/fate/filter-metadata-silencedetect | 2 +-
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/libavfilter/af_silencedetect.c b/libavfilter/af_silencedetect.c
index 3a71f39..2da8dbe 100644
--- a/libavfilter/af_silencedetect.c
+++ b/libavfilter/af_silencedetect.c
@@ -32,6 +32,8 @@
#include "avfilter.h"
#include "internal.h"
+const char TIMESTAMP_FMT_SILENCEDETECT[] = "%.6f";
+
typedef struct SilenceDetectContext {
const AVClass *class;
double noise; ///< noise amplitude ratio
@@ -86,11 +88,11 @@ static av_always_inline void update(SilenceDetectContext
*s, AVFrame *insamples,
s->start[channel] = insamples->pts +
av_rescale_q(current_sample / s->channels + 1 - nb_samples_notify *
s->independent_channels / s->channels,
(AVRational){ 1, s->last_sample_rate }, time_base);
set_meta(insamples, s->mono ? channel + 1 : 0, "silence_start",
-av_ts2timestr(s->start[channel], _base));
+av_ts2timestr_fmt(s->start[channel], _base,
TIMESTAMP_FMT_SILENCEDETECT));
if (s->mono)
av_log(s, AV_LOG_INFO, "channel: %d | ", channel);
av_log(s, AV_LOG_INFO, "silence_start: %s\n",
-av_ts2timestr(s->start[channel], _base));
+av_ts2timestr_fmt(s->start[channel], _base,
TIMESTAMP_FMT_SILENCEDETECT));
}
}
} else {
@@ -101,15 +103,15 @@ static av_always_inline void update(SilenceDetectContext
*s, AVFrame *insamples,
int64_t duration_ts = end_pts - s->start[channel];
if (insamples) {
set_meta(insamples, s->mono ? channel + 1 : 0, "silence_end",
-av_ts2timestr(end_pts, _base));
+av_ts2timestr_fmt(end_pts, _base,
TIMESTAMP_FMT_SILENCEDETECT));
Re: [FFmpeg-devel] [PATCH 14/21] avformat/matroskadec: Use proper levels after discontínuity
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
The earlier code set the level to zero upon seeking and after a
discontinuity although in both cases parsing (re)starts at a level 1
element.
Also set the segment's length to unkown if an error occured in order not
to drop any valid data that happens to be beyond the designated end of
the segment.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 59 +++
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 0179e5426e..42f1c21042 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -737,13 +737,24 @@ static const char *const matroska_doctypes[] = { "matroska",
"webm" };
static int matroska_read_close(AVFormatContext *s);
+static int matroska_reset_status(MatroskaDemuxContext *matroska,
+ uint32_t id, int64_t position)
+{
+matroska->current_id = id;
+matroska->num_levels = 1;
+matroska->current_cluster.pos = 0;
+
+if (position >= 0)
+return avio_seek(matroska->ctx->pb, position, SEEK_SET);
+
+return 0;
+}
+
I think you should have done this commit in 2 parts.
- adding matroska_reset_status() and do exactly what was done before
- add changes related to the level and unknown length/discontinuity.
static int matroska_resync(MatroskaDemuxContext *matroska, int64_t last_pos)
{
AVIOContext *pb = matroska->ctx->pb;
int64_t ret;
uint32_t id;
-matroska->current_id = 0;
-matroska->num_levels = 0;
/* seek to next position to resync from */
if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) {
@@ -759,7 +770,14 @@ static int matroska_resync(MatroskaDemuxContext *matroska,
int64_t last_pos)
id == MATROSKA_ID_CUES || id == MATROSKA_ID_TAGS||
id == MATROSKA_ID_SEEKHEAD || id == MATROSKA_ID_ATTACHMENTS ||
id == MATROSKA_ID_CLUSTER || id == MATROSKA_ID_CHAPTERS) {
-matroska->current_id = id;
+/* Prepare the context for further parsing of a level 1 element. */
+matroska_reset_status(matroska, id, -1);
You set num_levels to 1 now, leaving this function used to have
num_levels set to 0. I'm not sure it's correct or not.
+
+/* Given that we are here means that an error has occured,
I thought this function was meant to find a level1 ID after getting into
a Segment. This does not seem like an error at all.
+ * so treat the segment as unknown length in order not to
+ * discard valid data that happens to be beyond the designated
+ * end of the segment. */
+matroska->levels[0].length = EBML_UNKNOWN_LENGTH;
return 0;
}
id = (id << 8) | avio_r8(pb);
@@ -1610,18 +1628,12 @@ static int
matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
matroska->current_id = 0;
ret = ebml_parse(matroska, matroska_segment, matroska);
-
-/* remove dummy level */
-while (matroska->num_levels) {
-uint64_t length =
matroska->levels[--matroska->num_levels].length;
-if (length == EBML_UNKNOWN_LENGTH)
-break;
-}
I think this code indicates unknown length was handled in a seekhead
entry, probably because such files exist. Making the assumption in 13/21
about unknown length only on Segment+Cluster incorrect.
}
}
-/* seek back */
-avio_seek(matroska->ctx->pb, before_pos, SEEK_SET);
-matroska->current_id = saved_id;
+
+/* Seek back - notice that in all instances where this is used it is safe
+ * to set the level to 1 and unset the position of the current cluster. */
+matroska_reset_status(matroska, saved_id, before_pos);
return ret;
}
@@ -3535,9 +3547,7 @@ static int matroska_read_seek(AVFormatContext *s, int
stream_index,
timestamp = FFMAX(timestamp, st->index_entries[0].timestamp);
if ((index = av_index_search_timestamp(st, timestamp, flags)) < 0 || index == st->nb_index_entries - 1) {
-avio_seek(s->pb, st->index_entries[st->nb_index_entries - 1].pos,
- SEEK_SET);
-matroska->current_id = 0;
+matroska_reset_status(matroska, 0,
st->index_entries[st->nb_index_entries - 1].pos);
while ((index = av_index_search_timestamp(st, timestamp, flags)) < 0 ||
index == st->nb_index_entries - 1) {
matroska_clear_queue(matroska);
if (matroska_parse_cluster(matroska) < 0)
@@ -3557,8 +3567,8 @@ static int matroska_read_seek(AVFormatContext *s, int
stream_index,
tracks[i].end_timecode = 0;
}
-avio_seek(s->pb, st->index_entries[index].pos, SEEK_SET);
-matroska->current_id = 0;
+/* We seek to a level 1
Re: [FFmpeg-devel] [PATCH 13/21] avformat/matroskadec: Improve length check
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
The earlier code had three flaws:
1. The case of an unknown-sized element inside a finite-sized element
(which is against the specifications) was not caught.
2. The error message wasn't helpful: It compared the length of the child
with the offset of the end of the parent and claimed that the first
exceeds the latter, although that is not necessarily true.
3. Unknown-sized elements that are not parsed can't be skipped. Given
that according to the Matroska specifications only the segment and the
clusters can be of unknown-size, this is handled by not allowing any
This restriction is rather new. There might be old files that use
infinite sizes in other places.
other units to have infinite size whereas the earlier code would seek
back by 1 byte upon encountering an infinite-size element that ought
to be skipped.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 6fa324c0cc..0179e5426e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1180,11 +1180,29 @@ static int ebml_parse_elem(MatroskaDemuxContext
*matroska,
if (matroska->num_levels > 0) {
MatroskaLevel *level = >levels[matroska->num_levels -
1];
int64_t pos = avio_tell(pb);
-if (level->length != EBML_UNKNOWN_LENGTH &&
-(pos + length) > (level->start + level->length)) {
+
+if (length != EBML_UNKNOWN_LENGTH &&
+level->length != EBML_UNKNOWN_LENGTH) {
+uint64_t elem_end = pos + length,
+level_end = level->start + level->length;
+
+if (level_end < elem_end) {
+av_log(matroska->ctx, AV_LOG_ERROR,
+ "Element at 0x%"PRIx64" ending at 0x%"PRIx64" exceeds
"
+ "containing master element ending at 0x%"PRIx64"\n",
+ pos, elem_end, level_end);
+return AVERROR_INVALIDDATA;
+}
+} else if (level->length != EBML_UNKNOWN_LENGTH) {
+av_log(matroska->ctx, AV_LOG_ERROR, "Unknown-sized element "
+ "at 0x%"PRIx64" inside parent with finite size\n", pos);
+return AVERROR_INVALIDDATA;
IMO there's no reason to return an error here. You can log the error and
still parse the data, it should end up fine (if the code is done as such
that you can never read past your parents boundaries). At least libebml
can deal with that.
+} else if (length == EBML_UNKNOWN_LENGTH && id !=
MATROSKA_ID_CLUSTER) {
+// According to the specifications only clusters and segments
+// are allowed to be unknown-sized.
av_log(matroska->ctx, AV_LOG_ERROR,
- "Invalid length 0x%"PRIx64" > 0x%"PRIx64" in parent\n",
- length, level->start + level->length);
+ "Found unknown-sized element other than a cluster at "
+ "0x%"PRIx64". Dropping the invalid element.\n", pos);
return AVERROR_INVALIDDATA;
}
}
--
2.19.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 10/21] avformat/matroskadec: Don't keep old blocks
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
Before this commit, the Matroska muxer would read a block when required
to do so, parse the block, create and return the necessary AVPackets and
yet keep the blocks (in a dynamically allocated list), although they
aren't used at all any more. This has been changed. There is no list any
more and the block is immediately discarded after parsing.
My understanding of the code is that the blocks are put in a queue,
that's whatwebm_clusters_start_with_keyframe() uses to check that the
first frame is a keyframe (it doesn't check the type of the frame
though...). But since there's only one read
inmatroska_parse_cluster_incremental()there's only one kept in memory.
So LGTM.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 87 +--
1 file changed, 38 insertions(+), 49 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 9198fa1bea..997c96d78f 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -304,9 +304,20 @@ typedef struct MatroskaLevel {
uint64_t length;
} MatroskaLevel;
+typedef struct MatroskaBlock {
+uint64_t duration;
+int64_t reference;
+uint64_t non_simple;
+EbmlBin bin;
+uint64_t additional_id;
+EbmlBin additional;
+int64_t discard_padding;
+} MatroskaBlock;
+
typedef struct MatroskaCluster {
+MatroskaBlock block;
uint64_t timecode;
-EbmlList blocks;
+int64_t pos;
} MatroskaCluster;
typedef struct MatroskaLevel1Element {
@@ -356,8 +367,6 @@ typedef struct MatroskaDemuxContext {
MatroskaLevel1Element level1_elems[64];
int num_level1_elems;
-int current_cluster_num_blocks;
-int64_t current_cluster_pos;
MatroskaCluster current_cluster;
/* WebM DASH Manifest live flag */
@@ -367,16 +376,6 @@ typedef struct MatroskaDemuxContext {
int bandwidth;
} MatroskaDemuxContext;
-typedef struct MatroskaBlock {
-uint64_t duration;
-int64_t reference;
-uint64_t non_simple;
-EbmlBin bin;
-uint64_t additional_id;
-EbmlBin additional;
-int64_t discard_padding;
-} MatroskaBlock;
-
static const EbmlSyntax ebml_header[] = {
{ EBML_ID_EBMLREADVERSION,EBML_UINT, 0, offsetof(Ebml, version),
{ .u = EBML_VERSION } },
{ EBML_ID_EBMLMAXSIZELENGTH, EBML_UINT, 0, offsetof(Ebml, max_size),
{ .u = 8 } },
@@ -705,9 +704,9 @@ static const EbmlSyntax matroska_blockgroup[] = {
};
static const EbmlSyntax matroska_cluster_parsing[] = {
-{ MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,
offsetof(MatroskaCluster, timecode) },
-{ MATROSKA_ID_BLOCKGROUP, EBML_NEST, sizeof(MatroskaBlock),
offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
-{ MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, sizeof(MatroskaBlock),
offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
+{ MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster,
timecode) },
+{ MATROSKA_ID_BLOCKGROUP, EBML_NEST, 0, 0, { .n = matroska_blockgroup
} },
+{ MATROSKA_ID_SIMPLEBLOCK, EBML_PASS, 0, 0, { .n = matroska_blockgroup
} },
{ MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
{ MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
{ MATROSKA_ID_INFO,EBML_NONE },
@@ -3443,57 +3442,48 @@ end:
static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
{
-EbmlList *blocks_list;
-MatroskaBlock *blocks;
-int i, res;
+MatroskaCluster *cluster = >current_cluster;
+MatroskaBlock *block = >block;
+int res;
res = ebml_parse(matroska,
matroska_cluster_parsing,
- >current_cluster);
+ cluster);
if (res == 1) {
/* New Cluster */
-if (matroska->current_cluster_pos)
+if (cluster->pos)
ebml_level_end(matroska);
-ebml_free(matroska_cluster_parsing, >current_cluster);
-memset(>current_cluster, 0, sizeof(MatroskaCluster));
-matroska->current_cluster_num_blocks = 0;
-matroska->current_cluster_pos= avio_tell(matroska->ctx->pb);
+cluster->pos = avio_tell(matroska->ctx->pb);
/* sizeof the ID which was already read */
if (matroska->current_id)
-matroska->current_cluster_pos -= 4;
+cluster->pos -= 4;
res = ebml_parse(matroska,
matroska_clusters,
- >current_cluster);
+ cluster);
/* Try parsing the block again. */
if (res == 1)
res = ebml_parse(matroska,
matroska_cluster_parsing,
- >current_cluster);
+ cluster);
}
-if (!res &&
-matroska->current_cluster_num_blocks <
Re: [FFmpeg-devel] [PATCH 08/21] avformat/matroskadec: Improve error messages
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote:
ebml_read_num had a number of flaws:
1. The check for read errors/EOF was totally wrong. E.g. an EBML number
beginning with the invalid 0x00 would be considered a read error,
although it is just invalid data.
2. The check for read errors/EOF was done just once, after reading the
first byte of the EBML number. But errors/EOF can happen inbetween, of
course, and this wasn't checked.
3. There was no way to distinguish when EOF should be an error (because
the data has to be there) for which an error message should be emitted
and when it is not necessarily an error (namely during parsing of EBML
IDs). Such a possibility has been added and used.
Maybe the title of the patch should rather mention that it's fixing the
EOF handling when reading EBML ID/Length. The changed error messages is
a less important consequence.
Some useless initializations were also fixed.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskadec.c | 61 ++-
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index a6617a607b..aa2266384a 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -820,29 +820,19 @@ static int ebml_level_end(MatroskaDemuxContext *matroska)
* Returns: number of bytes read, < 0 on error
*/
static int ebml_read_num(MatroskaDemuxContext *matroska, AVIOContext *pb,
- int max_size, uint64_t *number)
+ int max_size, uint64_t *number, int eof_forbidden)
{
-int read = 1, n = 1;
-uint64_t total = 0;
+int read, n = 1;
+uint64_t total;
+int64_t pos;
-/* The first byte tells us the length in bytes - avio_r8() can normally
- * return 0, but since that's not a valid first ebmlID byte, we can
- * use it safely here to catch EOS. */
-if (!(total = avio_r8(pb))) {
-/* we might encounter EOS here */
-if (!avio_feof(pb)) {
-int64_t pos = avio_tell(pb);
-av_log(matroska->ctx, AV_LOG_ERROR,
- "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
- pos, pos);
-return pb->error ? pb->error : AVERROR(EIO);
-}
-return AVERROR_EOF;
-}
+/* The first byte tells us the length in bytes - except when it is zero. */
+total = avio_r8(pb);
+if (avio_feof(pb))
+goto err;
/* get the length of the EBML number */
-read = 8 - ff_log2_tab[total];
-if (read > max_size) {
+if (!total || (read = 8 - ff_log2_tab[total]) > max_size) {
int64_t pos = avio_tell(pb) - 1;
av_log(matroska->ctx, AV_LOG_ERROR,
"Invalid EBML number size tag 0x%02x at pos %"PRIu64"
(0x%"PRIx64")\n",
@@ -855,9 +845,27 @@ static int ebml_read_num(MatroskaDemuxContext *matroska,
AVIOContext *pb,
while (n++ < read)
total = (total << 8) | avio_r8(pb);
+if (avio_feof(pb)) {
+eof_forbidden = 1;
+goto err;
+}
You're forcing an error if the data ends after reading a number ? Ending
a Matroska file with a number should be fine. It could also be an
element with a size of 0. It doesn't contain any data but it's still
valid (depending on the semantic of the element).
So this forced error seem wrong. Let the next read catch the EOF if it
finds one.
+
*number = total;
return read;
+
+err:
+pos = avio_tell(pb);
+if (pb->error) {
+av_log(matroska->ctx, AV_LOG_ERROR,
+ "Read error at pos. %"PRIu64" (0x%"PRIx64")\n",
+ pos, pos);
+return pb->error;
+}
+if (eof_forbidden)
+av_log(matroska->ctx, AV_LOG_ERROR, "File ended prematurely "
+ "at pos. %"PRIu64" (0x%"PRIx64")\n", pos, pos);
If eof_forbidden is false (EOF allowed) you return an EOF error anyway ?
+return AVERROR_EOF;
}
/**
@@ -868,7 +876,7 @@ static int ebml_read_num(MatroskaDemuxContext *matroska,
AVIOContext *pb,
static int ebml_read_length(MatroskaDemuxContext *matroska, AVIOContext *pb,
uint64_t *number)
{
-int res = ebml_read_num(matroska, pb, 8, number);
+int res = ebml_read_num(matroska, pb, 8, number, 1);
if (res > 0 && *number + 1 == 1ULL << (7 * res))
*number = EBML_UNKNOWN_LENGTH;
return res;
@@ -1010,7 +1018,7 @@ static int matroska_ebmlnum_uint(MatroskaDemuxContext
*matroska,
{
AVIOContext pb;
ffio_init_context(, data, size, 0, NULL, NULL, NULL, NULL);
-return ebml_read_num(matroska, , FFMIN(size, 8), num);
+return ebml_read_num(matroska, , FFMIN(size, 8), num, 1);
}
/*
@@ -1057,7 +1065,7 @@ static int ebml_parse(MatroskaDemuxContext *matroska,
EbmlSyntax *syntax,
{
if (!matroska->current_id) {
uint64_t id;
-int res = ebml_read_num(matroska, matroska->ctx->pb, 4, );
[FFmpeg-devel] [PATCH v2] Added XV Support
This time i modified the initial buffer at the time of reading header
instead of changing the IO layer.
Suggest any changes required.
>From 277a4267f8cbb68c5fa57a0bddd215e04ca662bd Mon Sep 17 00:00:00 2001
From: Shivam Goyal
Date: Sun, 7 Apr 2019 12:13:21 +0530
Subject: [PATCH] Added XV Support
---
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/flvdec.c | 87
3 files changed, 89 insertions(+)
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 99be60d184..262df876e9 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -563,6 +563,7 @@ OBJS-$(CONFIG_XBIN_DEMUXER) += bintext.o sauce.o
OBJS-$(CONFIG_XMV_DEMUXER) += xmv.o
OBJS-$(CONFIG_XVAG_DEMUXER) += xvag.o
OBJS-$(CONFIG_XWMA_DEMUXER) += xwma.o
+OBJS-$(CONFIG_XV_DEMUXER)+= flvdec.o
OBJS-$(CONFIG_YOP_DEMUXER) += yop.o
OBJS-$(CONFIG_YUV4MPEGPIPE_DEMUXER) += yuv4mpegdec.o
OBJS-$(CONFIG_YUV4MPEGPIPE_MUXER)+= yuv4mpegenc.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index d316a0529a..b499186071 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -456,6 +456,7 @@ extern AVOutputFormat ff_wv_muxer;
extern AVInputFormat ff_xa_demuxer;
extern AVInputFormat ff_xbin_demuxer;
extern AVInputFormat ff_xmv_demuxer;
+extern AVInputFormat ff_xv_demuxer;
extern AVInputFormat ff_xvag_demuxer;
extern AVInputFormat ff_xwma_demuxer;
extern AVInputFormat ff_yop_demuxer;
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index b531a39adc..df732ca65e 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -127,6 +127,19 @@ static int kux_probe(const AVProbeData *p)
return 0;
}
+static int xv_probe(const AVProbeData *p)
+{
+const uint8_t *d = p->buf;
+
+if (d[0] == 'X' &&
+d[1] == 'L' &&
+d[2] == 'V' &&
+d[3] == 'F') {
+return AVPROBE_SCORE_MAX;
+}
+return 0;
+}
+
static void add_keyframes_index(AVFormatContext *s)
{
FLVContext *flv = s->priv_data;
@@ -459,6 +472,13 @@ static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc, int64_t m
}
}
+// For XV file the flv data starts from 0x20 byte
+if(!strcmp(s->iformat->name , "xv")){
+for (i=0; i < FFMIN(2,fileposlen); i++){
+filepositions[i] += 0x20;
+}
+}
+
if (timeslen == fileposlen && fileposlen>1 && max_pos <= filepositions[0]) {
for (i = 0; i < FFMIN(2,fileposlen); i++) {
flv->validate_index[i].pos = filepositions[i];
@@ -783,6 +803,52 @@ static int flv_read_header(AVFormatContext *s)
return 0;
}
+static int xv_read_header(AVFormatContext *s)
+{
+int flags;
+FLVContext *xv = s->priv_data;
+AVIOContext *ic = s->pb;
+int offset;
+int pre_tag_size = 0;
+int rot;
+
+//Find the rot value for rotating the bytes
+avio_seek(ic, 0x20, SEEK_SET);
+rot = 0x46 - avio_r8(ic);
+
+avio_skip(ic, 3);
+
+flags = (avio_r8(ic) + rot ) & 0xff;
+
+xv->missing_streams = flags & (FLV_HEADER_FLAG_HASVIDEO | FLV_HEADER_FLAG_HASAUDIO);
+
+s->ctx_flags |= AVFMTCTX_NOHEADER;
+
+offset = avio_r8(ic) + rot)&0xff)<<24) |
+(((avio_r8(ic) + rot)&0xff)<<16) |
+(((avio_r8(ic) + rot)&0xff)<<8) |
+(((avio_r8(ic) + rot)&0xff)))+ 0x20;
+
+avio_seek(ic, offset + 4, SEEK_SET);
+
+
+// Will modify the current buffer, as only
+// the bytes from 0x20 to 0x200400 are needed to decode
+int size = ic->buf_end - ic->buf_ptr;
+int64_t pos = ic->pos - size;
+for(int i=0;i<0x400; i++){
+if(pos >= 0x200400) break;
+(*(ic->buf_ptr + i)) = (((*(ic->buf_ptr + i)) + rot) & 0xff );
+pos+=1;
+}
+
+s->start_time = 0;
+xv->sum_flv_tag_size = 0;
+xv->last_keyframe_stream_index = -1;
+
+return 0;
+}
+
static int flv_read_close(AVFormatContext *s)
{
int i;
@@ -1424,3 +1490,24 @@ AVInputFormat ff_kux_demuxer = {
.extensions = "kux",
.priv_class = _class,
};
+
+static const AVClass xv_class = {
+.class_name = "xvdec",
+.item_name = av_default_item_name,
+.option = options,
+.version= LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_xv_demuxer = {
+.name = "xv",
+.long_name = NULL_IF_CONFIG_SMALL("Xunlie Video File"),
+.priv_data_size = sizeof(FLVContext),
+.read_probe = xv_probe,
+.read_header= xv_read_header,
+.read_packet= flv_read_packet,
+.read_seek = flv_read_seek,
+.read_close = flv_read_close,
+.extensions = "xv",
+.priv_class = _class,
+.flags = AVFMT_TS_DISCONT
+};
--
2.21.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
Re: [FFmpeg-devel] [PATCH 05/21] avformat/matroskadec: Set offset of first cluster
On 3/27/2019 12:18 PM, Andreas Rheinhardt via ffmpeg-devel wrote: By default, the data_offset member of the AVFormatInternal of the AVFormatContext associated with the MatroskaDemuxContext has not been initialized explicitly by any Matroska-specific function, so that it was initialized by default to the offset at the end of matroska_read_header, i.e. usually to the offset of the length field of the first encountered cluster. This meant that in case that the Matroska-specific seek-code fails because there are no index entries for the target track a seek to data_offset would be performed and ordinary parsing would start from there which is nonsense: The length field would be treated as EBML ID and (if the length field is not longer than four bytes (EBML numbers that long are rejected as invalid EBML IDs)) and whatever comes next would be treated as its EBML size although it simply isn't. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskadec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 49f8ff4082..f9811b54a1 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -2651,6 +2651,9 @@ static int matroska_read_header(AVFormatContext *s) pos = avio_tell(matroska->ctx->pb); res = ebml_parse(matroska, matroska_segment, matroska); } +/* Set data_offset as it might be needed later by seek_frame_generic. */ +if (matroska->current_id) I'm surprised this doesn't error out if a (level 1) ID is not found here. +s->internal->data_offset = avio_tell(matroska->ctx->pb) - 4; The "- 4" is OK as long as level 1 elements are always 4 bytes (which is the case). But if matroska_resync() ever exits if it finds an EBML Void or CRC-32 then this will break. The code is safe for now but may not be future proof. matroska_execute_seekhead(matroska); if (!matroska->time_scale) -- 2.19.2 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] swscale/ppc: VSX-optimize yuv2rgb_full_X
On Mon, 1 Apr 2019 13:37:32 +0300 Lauri Kasanen wrote: > ./ffmpeg -f lavfi -i yuvtestsrc=duration=1:size=1200x1440 \ > -s 1200x720 -f null -vframes 100 -pix_fmt $i -nostats \ > -cpuflags 0 -v error - > > 32-bit mul, power8 only. > > ~6.4x speedup: > > rgb24 > 214278 UNITS in yuv2packedX, 16384 runs, 0 skips > 33249 UNITS in yuv2packedX, 16384 runs, 0 skips > bgr24 > 214616 UNITS in yuv2packedX, 16384 runs, 0 skips > 33233 UNITS in yuv2packedX, 16384 runs, 0 skips > rgba > 214517 UNITS in yuv2packedX, 16384 runs, 0 skips > 33271 UNITS in yuv2packedX, 16384 runs, 0 skips > bgra > 214973 UNITS in yuv2packedX, 16384 runs, 0 skips > 33397 UNITS in yuv2packedX, 16384 runs, 0 skips > argb > 214613 UNITS in yuv2packedX, 16384 runs, 0 skips > 33310 UNITS in yuv2packedX, 16384 runs, 0 skips > bgra > 214637 UNITS in yuv2packedX, 16384 runs, 0 skips > 0 UNITS in yuv2packedX, 16384 runs, 0 skips > > Signed-off-by: Lauri Kasanen > --- > libswscale/ppc/swscale_vsx.c | 160 > +++ > 1 file changed, 160 insertions(+) Applying. - Lauri ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] swscale/ppc: VSX-optimize yuv2rgb_full_2
On Mon, 1 Apr 2019 13:13:59 +0300 Lauri Kasanen wrote: > ./ffmpeg -f lavfi -i yuvtestsrc=duration=1:size=1200x1440 -sws_flags area \ > -s 1200x720 -f null -vframes 100 -pix_fmt $i -nostats \ > -cpuflags 0 -v error - > > 32-bit mul, power8 only. > > ~4x speedup: > > rgb24 > 52763 UNITS in yuv2packed2, 16384 runs, 0 skips > 13453 UNITS in yuv2packed2, 16384 runs, 0 skips > bgr24 > 53144 UNITS in yuv2packed2, 16384 runs, 0 skips > 13616 UNITS in yuv2packed2, 16384 runs, 0 skips > rgba > 52796 UNITS in yuv2packed2, 16384 runs, 0 skips > 12904 UNITS in yuv2packed2, 16384 runs, 0 skips > bgra > 52732 UNITS in yuv2packed2, 16384 runs, 0 skips > 13262 UNITS in yuv2packed2, 16384 runs, 0 skips > argb > 52661 UNITS in yuv2packed2, 16384 runs, 0 skips > 12879 UNITS in yuv2packed2, 16384 runs, 0 skips > bgra > 52662 UNITS in yuv2packed2, 16384 runs, 0 skips > 12932 UNITS in yuv2packed2, 16384 runs, 0 skips > > Signed-off-by: Lauri Kasanen > --- > libswscale/ppc/swscale_vsx.c | 166 > +++ > 1 file changed, 166 insertions(+) Applying. - Lauri ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] swscale/ppc: VSX-optimize non-full-chroma yuv2rgb_1
On Sun, 31 Mar 2019 17:18:47 +0300 Lauri Kasanen wrote: > ./ffmpeg -f lavfi -i yuvtestsrc=duration=1:size=1200x1440 -sws_flags > fast_bilinear \ > -s 1200x1440 -f null -vframes 100 -pix_fmt $i -nostats \ > -cpuflags 0 -v error - > > 32-bit mul, power8 only. > > 1.8-2.3x speedup: > > rgb24 > 18192 UNITS in yuv2packed1, 32767 runs, 1 skips >9983 UNITS in yuv2packed1, 32760 runs, 8 skips > bgr24 > 18665 UNITS in yuv2packed1, 32766 runs, 2 skips >9925 UNITS in yuv2packed1, 32763 runs, 5 skips > rgba > 20239 UNITS in yuv2packed1, 32767 runs, 1 skips >8794 UNITS in yuv2packed1, 32759 runs, 9 skips > bgra > 20354 UNITS in yuv2packed1, 32768 runs, 0 skips >8770 UNITS in yuv2packed1, 32761 runs, 7 skips > argb > 20185 UNITS in yuv2packed1, 32768 runs, 0 skips >8761 UNITS in yuv2packed1, 32761 runs, 7 skips > bgra > 20360 UNITS in yuv2packed1, 32766 runs, 2 skips >8759 UNITS in yuv2packed1, 32764 runs, 4 skips > > This is a low speedup, but the x86 mmx version also gets only ~2x. The mmx > version > is also heavily inaccurate, while the vsx version has high accuracy. Applying. - Lauri ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 00/21] New Version
Hi, On 3/27/2019 12:18 PM, Andreas Rheinhardt wrote: This also changed the handling of unknown-sized elements: They are now ended whenever an element not known to be allowed in them is encountered. If we are already on level 1 and encounter an element not known to be allowed in an unknown-sized segment, this is treated as an indication that an error might have occured. I hope this is fine. I haven't looked at the code yet, but an unknown element doesn't mean it's an upper level element. I think it should be either skipped or considered as bad data. If it's a known element but complitely misplaced it should not be going upper in the hierarchy. Only when a valid upper element it should go up in the hierarchy. Dale's sample "bear-320x240-live.webm" btw has cues at the end that use unknown-sized elements (wastefully coded on eight bytes) for CuePoints and CueTrackPositions which is spec-incompliant. They are not referenced by a SeekHead and so can't be used for seeking, but if they were, neither current FFmpeg nor FFmpeg with my patches applied would be able to use them. Are such files common (this is the first such file I ever saw)? If so, I could probably make it work. If CuePoints are not referenced by the SeekHead at the front of the file (or the indirect SeekHead) they are useless anyway. I have cced Steve for this (I didn't the first time, because I thought that he (as a maintainer) would also be a subscriber to this list). I am subscribed but not the maintainer. In fact I know little about this code. Oh, and I did not check with Valgrind that the new lacing code doesn't read uninitialized data. I don't even know how to use Valgrind. I just read the code. If someone more knowledgeable than I could please test it... You might also use LLVM with ASAN (address sanitizer) it's helpfull for this (or so I have been told). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
