[FFmpeg-devel] [PATCH] dpx.c: Better support for reading fps from headers

[Michael Niedermayer]

On Fri, Jul 04, 2014 at 03:01:25AM -0600, Bob Maple wrote:
> Relatively simple patch to better support fetching the fps from DPX files.
> 
> DPX has 2 separate "industry" headers -- both which are required, but
> not to actually be legitimately filled with anything.  Currently ffmpeg
> only looks in the Film header to get the FPS, but many industry apps
> write into the TV header since it's more applicable.  Some write to
> both...  I've found the DPX landscape to be the wild wild west, all
> sorts of abuse of the standard (which its self is vague in several places.)
> 
> In any case, this looks in the Film header first then tries the TV
> header.  Nothing too crazy.  I'd like to add timecode support next but
> this is my first real foray into the ffmpeg source and I'm still totally
> lost as to the flow of things.
> 
> (helps if I actually attach this time..)

>  dpx.c |   21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> a02cfeb48e750dde2bc1bc4e1fe5d150852085e9  0001-dpx.c-Better-support-for-reading-fps-from-headers.patch
> From b5db33ff18db6b0bae07968bd3f6dcf986956804 Mon Sep 17 00:00:00 2001
> From: Bob Maple <bobm-ffdev at burner.com>
> Date: Thu, 3 Jul 2014 17:30:53 -0600
> Subject: [PATCH] dpx.c: Better support for reading fps from headers
> 
> ---
>  libavcodec/dpx.c |   21 ++++++++++++++-------
>  1 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
> index 5f05cd8..acda974 100644
> --- a/libavcodec/dpx.c
> +++ b/libavcodec/dpx.c
> @@ -147,14 +147,21 @@ static int decode_frame(AVCodecContext *avctx,
>      else
>          avctx->sample_aspect_ratio = (AVRational){ 0, 1 };
>  
> -    if (offset >= 1724 + 4) {
> -        buf = avpkt->data + 1724;
> +    // Look in the 'Film Industry' header and the 'TV Industry' header for fps
> +    buf = avpkt->data + 1724;
> +    i = read32(&buf, endian);

you are removing an out of array access check
this is not ok unless theres some other check somewhere that makes
it redundant


> +
> +    // All undefined fields are supposed to be set to all 0xFF
> +    // but some writers use +inf here
> +    if (i == 0x7F800000 || i == 0xFFFFFFFF) {
> +        buf = avpkt->data + 1940;
>          i = read32(&buf, endian);

missing check that this is within the input array


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140704/d6d491cf/attachment.asc>