[FFmpeg-devel] [RFC/PATCH] Pass PRIVATE_STREAM_2 MPEG-PS packets to caller

Michael Niedermayer michaelni at gmx.at
Sat Mar 2 13:18:31 CET 2013


On Tue, Feb 26, 2013 at 05:45:23PM +0100, Richard wrote:
> On 25/02/13 17:23, Richard wrote:
> >
> >If you are ok with my suggestions above, I'll create a new patch to
> >parse and merge the packets, setting the fields as required.
> 
> Assuming no reply is an 'ok', I've attached a new patch.  This patch
> only passes the two packets as used on DVDs with the codec
> AV_CODEC_ID_DVD_NAV.  The packets are combined and the pts and
> duration fields of the AVPacket are set based on the contents of the
> PCI packet.
> 
> Richard.
> 

>  configure                   |    2 -
>  libavcodec/Makefile         |    1 
>  libavcodec/allcodecs.c      |    1 
>  libavcodec/avcodec.h        |    2 +
>  libavcodec/codec_desc.c     |    6 +++
>  libavcodec/dvd_nav_parser.c |   85 ++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h        |    2 -
>  libavformat/mpeg.c          |   28 +++++++++++---
>  8 files changed, 120 insertions(+), 7 deletions(-)
> cd59250a2af4984fb9353db52f3b314dbc95d357  0001-Add-passing-DVD-navigation-packets-startcode-0x1bf-t.patch
> From eea790f8175403703be1559f8d725140ca3987bd Mon Sep 17 00:00:00 2001
> From: Richard <peper03 at yahoo.com>
> Date: Tue, 26 Feb 2013 17:31:42 +0100
> Subject: [PATCH] Add passing DVD navigation packets (startcode 0x1bf) to
>  caller to allow better playback handling of DVDs.  The two
>  types of packets (PCI and DSI) are passed untouched but
>  combined by the new codec ID AV_CODEC_ID_DVD_NAV.  The
>  first 980 bytes in the packet contain the PCI data.  The
>  next 1018 are the DSI data.
> 
> ---
>  configure                   |    2 +-
>  libavcodec/Makefile         |    1 +
>  libavcodec/allcodecs.c      |    1 +
>  libavcodec/avcodec.h        |    2 +
>  libavcodec/codec_desc.c     |    6 +++
>  libavcodec/dvd_nav_parser.c |   85 +++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h        |    2 +-
>  libavformat/mpeg.c          |   28 +++++++++++---
>  8 files changed, 120 insertions(+), 7 deletions(-)
>  create mode 100644 libavcodec/dvd_nav_parser.c
> 
> diff --git a/configure b/configure
> index f442715..52c3078 100755
> --- a/configure
> +++ b/configure
> @@ -1869,7 +1869,7 @@ wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
>  # parsers
>  h264_parser_select="error_resilience golomb h264chroma h264dsp h264pred h264qpel mpegvideo videodsp"
>  mpeg4video_parser_select="error_resilience mpegvideo"
> -mpegvideo_parser_select="error_resilience mpegvideo"
> +mpegvideo_parser_select="error_resilience mpegvideo dvdnav"
>  vc1_parser_select="error_resilience mpegvideo"

this looks wrong, maybe you meant dvdnav_parser but either way make
sure the dependancies have been tested if you are unsure if they are
correct
 
also dont mix dvdnav and dvd_nav it will not work


>  
>  # external libraries
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 52282e3..3b8b277 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -741,6 +741,7 @@ OBJS-$(CONFIG_PNG_PARSER)              += png_parser.o
>  OBJS-$(CONFIG_MPEGAUDIO_PARSER)        += mpegaudio_parser.o \
>                                            mpegaudiodecheader.o mpegaudiodata.o
>  OBJS-$(CONFIG_MPEGVIDEO_PARSER)        += mpegvideo_parser.o    \
> +                                          dvd_nav_parser.o \
>                                            mpeg12.o mpeg12data.o
>  OBJS-$(CONFIG_PNM_PARSER)              += pnm_parser.o pnm.o
>  OBJS-$(CONFIG_RV30_PARSER)             += rv34_parser.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 584446f..1eaf2d3 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -515,6 +515,7 @@ void avcodec_register_all(void)
>      REGISTER_PARSER(DNXHD,              dnxhd);
>      REGISTER_PARSER(DVBSUB,             dvbsub);
>      REGISTER_PARSER(DVDSUB,             dvdsub);
> +    REGISTER_PARSER(DVD_NAV,            dvd_nav);
>      REGISTER_PARSER(FLAC,               flac);
>      REGISTER_PARSER(GSM,                gsm);
>      REGISTER_PARSER(H261,               h261);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index f809e3d..1e08271 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -483,6 +483,8 @@ enum AVCodecID {
>      AV_CODEC_ID_IDF        = MKBETAG( 0 ,'I','D','F'),
>      AV_CODEC_ID_OTF        = MKBETAG( 0 ,'O','T','F'),
>      AV_CODEC_ID_SMPTE_KLV  = MKBETAG('K','L','V','A'),
> +    AV_CODEC_ID_DVD_NAV    = MKBETAG('D','N','A','V'),
> +
>  
>      AV_CODEC_ID_PROBE = 0x19000, ///< codec_id is not known (like AV_CODEC_ID_NONE) but lavf should attempt to identify it
>  
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 440e9d9..32db185 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -2523,6 +2523,12 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "klv",
>          .long_name = NULL_IF_CONFIG_SMALL("SMPTE 336M Key-Length-Value (KLV) metadata"),
>      },
> +    {
> +        .id        = AV_CODEC_ID_DVD_NAV,
> +        .type      = AVMEDIA_TYPE_DATA,
> +        .name      = "dvd_nav_packet",
> +        .long_name = NULL_IF_CONFIG_SMALL("DVD Nav packet"),
> +    },
>  
>  };
>  
> diff --git a/libavcodec/dvd_nav_parser.c b/libavcodec/dvd_nav_parser.c
> new file mode 100644
> index 0000000..21deb45
> --- /dev/null
> +++ b/libavcodec/dvd_nav_parser.c
> @@ -0,0 +1,85 @@
> +/*
> + * DVD navigation block parser for FFmpeg
> + * Copyright (c) 2013 The ffmpeg Project
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "avcodec.h"
> +#include "dsputil.h"
> +#include "get_bits.h"
> +#include "parser.h"
> +
> +/* parser definition */
> +typedef struct DVDNavParseContext {
> +    ParseContext pc;
> +} DVDNavParseContext;
> +
> +static int dvd_nav_parse(AVCodecParserContext *s,
> +                         AVCodecContext *avctx,
> +                         const uint8_t **poutbuf, int *poutbuf_size,
> +                         const uint8_t *buf, int buf_size)
> +{
> +    DVDNavParseContext *pc1 = s->priv_data;
> +    ParseContext *pc= &pc1->pc;
> +    int next = END_NOT_FOUND;
> +    int blockOK = 1;
> +
> +    s->pict_type = AV_PICTURE_TYPE_NONE;
> +
> +    avctx->time_base.num = 1;
> +    avctx->time_base.den = 90000;
> +
> +    switch(buf[0])
> +    {
> +        case 0x00:
> +        {
> +            /* PCI */
> +            uint32_t startpts = AV_RB32(&buf[0x0D]);
> +            uint32_t endpts = AV_RB32(&buf[0x11]);

possible out of array accesses


> +            s->pts = (int64_t)startpts;
> +            s->duration = endpts - startpts;

missing validity checks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130302/a8c8906b/attachment.asc>


More information about the ffmpeg-devel mailing list