[FFmpeg-devel] [PATCH] [WIP] [RFC] dvdvideo: initial contribution (DVD demuxer)

Marth64 marth64 at proxyid.net
Sun Dec 10 05:47:22 EET 2023


> What is a yuv-to-rgb function doing inside a dvd-video demuxer?
DVD subtitle palette is YUV. However, when stored in modern formats such as
matroska or MP4, the "convention" is to store in RGB.
In fact, mp4/mov muxer has a similar routine.

> This function is pointless.
I'm opening the MPEG-PS subdemuxer with custom IO flag. It seems to expect
this according to the documentation. I can try without.

On Sat, Dec 9, 2023 at 9:03 PM Leo Izen <leo.izen at gmail.com> wrote:

> On 12/9/23 05:06, Marth64 wrote:
> > Hello, I am happy to share a DVD demuxer for ffmpeg powered by
> libdvdread and libdvdnav.
> > I have been working on this on/off throughout the year and think it is
> in a good spot
> > to share at the ML now. This was a major learning experience for me in
> many ways and
> > am open to any feedback on how I could improve it to be better. In fact,
> there is still
> > one issue I can't seem to figure out how to solve (in discussion below).
> Regardless,
> > a good number of DVDs I have tried seem to work out of the box. This is
> a full-service
> > demuxer with chapters, subtitles (and their palettes), as well as
> language metadata.
> >
> > At a high level, the demuxer uses libdvdread for metadata of the disc,
> libdvdnav for
> > the actual playback, and the MPEG-PS demuxer for the underlying VOB
> stream.
> >
> > First, the basic usage, is quite straightforward:
> > ffmpeg -f dvdvideo -title NN -i DVD.iso|/dev/srX|/path/to/DVD ...
> > Where NN can be replaced by your known title number in the disc
> >
> > As the demuxer effectively works at a PGC level, multi-PGC titles are
> not supported.
> > But to provide this flexibility as there are many weirdly authored DVDs
> out there,
> > one can specify an exact PGC via -pgc option and an associated program
> (PG, can start at 1).
> >
> > I am hoping and willing to improve this to be a robust demuxer wherever
> possible, but to
> > that extent there is still an issue I can't figure out how to solve:
> Dealing with DTS discontinuities
> > and PTS generation.
> >
> > Right now, as a band-aid, I am adding AVFMT_FLAG_GENPTS (line 1408)
> > to the MPEG-PS subdemuxer. This works with most discs and the output
> seems OK. On discs with discontinuities, however,
> > this causes the demuxing to hang which is obviously unacceptable also.
> Removing the flag causes the discs
> > to not hang, but then the output for all discs becomes choppy for
> obvious reasons (invalid PTS).
> >
> > It could be because I have some misunderstandings on how things work
> within ffmpeg,
> > but I have tried to the point now where I thought it best to ask the
> experts for help if you can spot
> > what I am doing wrong. I am really motivated to make this work and good
> quality.
> >
> > Thank you!
> >
> > ---
> >   configure                 |    8 +
> >   libavformat/Makefile      |    1 +
> >   libavformat/allformats.c  |    1 +
> >   libavformat/avlanguage.c  |   10 +-
> >   libavformat/dvdvideodec.c | 1599 +++++++++++++++++++++++++++++++++++++
> >   5 files changed, 1617 insertions(+), 2 deletions(-)
> >   create mode 100644 libavformat/dvdvideodec.c
> >
> > diff --git a/configure b/configure
> > index d77c053226..65e5968194 100755
> > --- a/configure
> > +++ b/configure
> > @@ -227,6 +227,8 @@ External library support:
> >     --enable-libdavs2        enable AVS2 decoding via libdavs2 [no]
> >     --enable-libdc1394       enable IIDC-1394 grabbing using libdc1394
> >                              and libraw1394 [no]
> > +  --enable-libdvdnav       enable libdvdnav, needed for DVD demuxing
> [no]
> > +  --enable-libdvdread      enable libdvdread, needed for DVD demuxing
> [no]
>
> Why do you need both of these? libdvdnav depends on libdvdread.
>
> >     --enable-libfdk-aac      enable AAC de/encoding via libfdk-aac [no]
> >     --enable-libflite        enable flite (voice synthesis) support via
> libflite [no]
> >     --enable-libfontconfig   enable libfontconfig, useful for drawtext
> filter [no]
> > @@ -1802,6 +1804,8 @@ EXTERNAL_LIBRARY_GPL_LIST="
> >       frei0r
> >       libcdio
> >       libdavs2
> > +    libdvdnav
> > +    libdvdread
> >       librubberband
> >       libvidstab
> >       libx264
> > @@ -3494,6 +3498,8 @@ dts_demuxer_select="dca_parser"
> >   dtshd_demuxer_select="dca_parser"
> >   dv_demuxer_select="dvprofile"
> >   dv_muxer_select="dvprofile"
> > +dvdvideo_demuxer_select="mpegps_demuxer"
> > +dvdvideo_demuxer_deps="libdvdnav libdvdread"
> >   dxa_demuxer_select="riffdec"
> >   eac3_demuxer_select="ac3_parser"
> >   evc_demuxer_select="evc_frame_merge_bsf evc_parser"
> > @@ -6723,6 +6729,8 @@ enabled libdav1d          && require_pkg_config
> libdav1d "dav1d >= 0.5.0" "dav1d
> >   enabled libdavs2          && require_pkg_config libdavs2 "davs2 >=
> 1.6.0" davs2.h davs2_decoder_open
> >   enabled libdc1394         && require_pkg_config libdc1394 libdc1394-2
> dc1394/dc1394.h dc1394_new
> >   enabled libdrm            && require_pkg_config libdrm libdrm
> xf86drm.h drmGetVersion
> > +enabled libdvdnav         && require_pkg_config libdvdnav "dvdnav >=
> 6.1.1" dvdnav/dvdnav.h dvdnav_open2
> > +enabled libdvdread        && require_pkg_config libdvdread "dvdread >=
> 6.1.2" dvdread/dvd_reader.h DVDOpen2 -ldvdread
> >   enabled libfdk_aac        && { check_pkg_config libfdk_aac fdk-aac
> "fdk-aac/aacenc_lib.h" aacEncOpen ||
> >                                  { require libfdk_aac
> fdk-aac/aacenc_lib.h aacEncOpen -lfdk-aac &&
> >                                    warn "using libfdk without
> pkg-config"; } }
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > index 2db83aff81..45dba53044 100644
> > --- a/libavformat/Makefile
> > +++ b/libavformat/Makefile
> > @@ -192,6 +192,7 @@ OBJS-$(CONFIG_DTS_MUXER)                 += rawenc.o
> >   OBJS-$(CONFIG_DV_MUXER)                  += dvenc.o
> >   OBJS-$(CONFIG_DVBSUB_DEMUXER)            += dvbsub.o rawdec.o
> >   OBJS-$(CONFIG_DVBTXT_DEMUXER)            += dvbtxt.o rawdec.o
> > +OBJS-$(CONFIG_DVDVIDEO_DEMUXER)          += dvdvideodec.o
> >   OBJS-$(CONFIG_DXA_DEMUXER)               += dxa.o
> >   OBJS-$(CONFIG_EA_CDATA_DEMUXER)          += eacdata.o
> >   OBJS-$(CONFIG_EA_DEMUXER)                += electronicarts.o
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > index c8bb4e3866..dc2acf575c 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -150,6 +150,7 @@ extern const AVInputFormat  ff_dv_demuxer;
> >   extern const FFOutputFormat ff_dv_muxer;
> >   extern const AVInputFormat  ff_dvbsub_demuxer;
> >   extern const AVInputFormat  ff_dvbtxt_demuxer;
> > +extern const AVInputFormat  ff_dvdvideo_demuxer;
> >   extern const AVInputFormat  ff_dxa_demuxer;
> >   extern const AVInputFormat  ff_ea_demuxer;
> >   extern const AVInputFormat  ff_ea_cdata_demuxer;
> > diff --git a/libavformat/avlanguage.c b/libavformat/avlanguage.c
> > index 782a58adb2..202d9aa835 100644
> > --- a/libavformat/avlanguage.c
> > +++ b/libavformat/avlanguage.c
> > @@ -29,7 +29,7 @@ typedef struct LangEntry {
> >       uint16_t next_equivalent;
> >   } LangEntry;
> >
> > -static const uint16_t lang_table_counts[] = { 484, 20, 184 };
> > +static const uint16_t lang_table_counts[] = { 484, 20, 190 };
> >   static const uint16_t lang_table_offsets[] = { 0, 484, 504 };
> >
> >   static const LangEntry lang_table[] = {
> > @@ -539,7 +539,7 @@ static const LangEntry lang_table[] = {
> >       /*0501*/ { "slk",  647 },
> >       /*0502*/ { "sqi",  652 },
> >       /*0503*/ { "zho",  686 },
> > -    /*----- AV_LANG_ISO639_1 entries (184) -----*/
> > +    /*----- AV_LANG_ISO639_1 entries (190) -----*/
> >       /*0504*/ { "aa" ,    0 },
> >       /*0505*/ { "ab" ,    1 },
> >       /*0506*/ { "ae" ,   33 },
> > @@ -724,6 +724,12 @@ static const LangEntry lang_table[] = {
> >       /*0685*/ { "za" ,  478 },
> >       /*0686*/ { "zh" ,   78 },
> >       /*0687*/ { "zu" ,  480 },
> > +    /*0688*/ { "in" ,  195 }, /* deprecated */
> > +    /*0689*/ { "iw" ,  172 }, /* deprecated */
> > +    /*0690*/ { "ji" ,  472 }, /* deprecated */
> > +    /*0691*/ { "jw" ,  202 }, /* deprecated */
> > +    /*0692*/ { "mo" ,  358 }, /* deprecated */
> > +    /*0693*/ { "sh" ,  693 }, /* deprecated (no equivalent) */
> >       { "", 0 }
> >   };
> >
> > diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> > new file mode 100644
> > index 0000000000..2b22a7de4f
> > --- /dev/null
> > +++ b/libavformat/dvdvideodec.c
> > @@ -0,0 +1,1599 @@
> > +/*
> > + * DVD-Video demuxer (powered by libdvdnav/libdvdread)
> > + *
> > + * 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
> > + */
> > +
> > +/**
> > + * DVD-Video is not a directly accessible, linear container format in
> the
> > + * traditional sense. Instead, it allows for complex and programmatic
> > + * playback of carefully muxed streams. A typical DVD player relies on
> > + * user GUI interaction to drive the direction of the demuxing.
> > + * Ultimately, the logical playback sequence is defined by a title's PGC
> > + * and a user selected "angle". An additional layer of control is
> defined by
> > + * NAV packets in the MPEG-PS, but as these are processed by libdvdnav,
> > + * they are witheld from the output of this demuxer.
> > + *
> > + * Therefore, the high-level approach is as follows:
> > + * 1) Open the volume with libdvdread
> > + * 2) Gather information about the user-requested title and PGC
> coordinates
> > + * 3) Request playback at the coordinates and chosen angle with
> libdvdnav
> > + * 4) Seek playback to first cell at the coordinates (skipping stills,
> etc.)
> > + * 5) Begin the playback (reading and demuxing) of MPEG-PS blocks
> > + * 6) End playback if the PGC or angle change
> > + * 7) Close resources
> > + **/
> > +
> > +/**
> > + * Issues/bug tracker (TODO):
> > + * - SDDS is not supported yet
> > + * - Are we handling backwards cell changes and PTT changes correctly?
> > + * - Some codec parameters/metadata is not being explicitly set:
> > + *      -> ChannelLayout, color/chroma info, DAR, frame size for
> MP1/MP2 audio
> > + * - Additional PGC validations?
> > +**/
> > +
> > +#include <dvdread/dvd_reader.h>
> > +#include <dvdread/ifo_read.h>
> > +#include <dvdread/ifo_types.h>
> > +#include <dvdread/nav_read.h>
> > +#include <dvdnav/dvdnav.h>
> > +
> > +#include "libavutil/avstring.h"
> > +#include "libavutil/avutil.h"
> > +#include "libavutil/colorspace.h"
> > +#include "libavutil/mem.h"
> > +#include "libavutil/opt.h"
> > +#include "libavutil/samplefmt.h"
> > +
> > +#include "libavcodec/avcodec.h"
> > +#include "libavformat/avio_internal.h"
> > +#include "libavformat/avlanguage.h"
> > +#include "libavformat/avformat.h"
> > +#include "libavformat/demux.h"
> > +#include "libavformat/internal.h"
> > +#include "libavformat/url.h"
> > +
> > +#define ZERO_Q                                          (AVRational) {
> 0, 1 }
> Unnecessary #define.
>
> > +
> > +#define DVDVIDEO_PS_MAX_SEARCH_BLOCKS                   128
> > +#define DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE                1024
> > +
> > +#define DVDVIDEO_TIME_BASE_Q                            (AVRational) {
> 1, 90000 }
> > +#define DVDVIDEO_TIME_BASE_MILLIS_Q                     (AVRational) {
> 1, 1000 }
> > +#define DVDVIDEO_PTS_WRAP_BITS                          32
> > +#define DVDVIDEO_BLOCK_SIZE                             2048
> > +
> > +#define DVDVIDEO_NTSC_FRAMERATE_Q                       (AVRational) {
> 30000, 1001 }
> > +#define DVDVIDEO_NTSC_HEIGHT                            480
> > +#define DVDVIDEO_PAL_FRAMERATE_Q                        (AVRational) {
> 25, 1 }
> > +#define DVDVIDEO_PAL_HEIGHT                             576
> > +#define DVDVIDEO_D1_WIDTH                               720
> > +#define DVDVIDEO_4CIF_WIDTH                             704
> > +#define DVDVIDEO_D1_HALF_WIDTH                          352
> > +#define DVDVIDEO_CIF_WIDTH                              352
> > +#define DVDVIDEO_PIXEL_FORMAT
>  AV_PIX_FMT_YUV420P
> > +
> > +#define DVDVIDEO_STARTCODE_VIDEO                        0x1E0
> > +#define DVDVIDEO_STARTCODE_OFFSET_AUDIO_AC3             0x80
> > +#define DVDVIDEO_STARTCODE_OFFSET_AUDIO_MP1             0x1C0
> > +#define DVDVIDEO_STARTCODE_OFFSET_AUDIO_MP2             0x1C0
> > +#define DVDVIDEO_STARTCODE_OFFSET_AUDIO_PCM             0xA0
> > +#define DVDVIDEO_STARTCODE_OFFSET_AUDIO_DTS             0x88
> > +#define DVDVIDEO_STARTCODE_OFFSET_SUBP                  0x20
> > +
> > +#define DVDVIDEO_TITLE_NUMBER_MAX                       99
> > +#define DVDVIDEO_PTT_NUMBER_MAX                         99
> > +#define DVDVIDEO_PGC_NUMBER_MAX                         32767
> > +#define DVDVIDEO_PG_NUMBER_MAX                          255
> > +#define DVDVIDEO_ANGLE_NUMBER_MAX                       9
> > +#define DVDVIDEO_VTS_NUMBER_MAX                         99
> > +#define DVDVIDEO_TT_NUMBER_MAX                          99
> > +#define DVDVIDEO_REGION_NUMBER_MAX                      8
> > +#define DVDVIDEO_AUDIO_CONTROL_ENABLED_MASK             0x8000
> > +#define DVDVIDEO_SUBP_CONTROL_ENABLED_MASK              0x80000000
> > +#define DVDVIDEO_VTS_AUDIO_STREAMS_MAX                  8
> > +#define DVDVIDEO_VTS_SUBP_STREAMS_MAX                   32
> > +
> > +/* ("palette: ") + ("rrggbb, "*15) + ("rrggbb") + \n + \0 */
> > +#define DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE           (9 + (8 * 15) +
> 6 + 1 + 1)
> > +#define DVDVIDEO_SUBP_CLUT_LEN                          16
> > +#define DVDVIDEO_SUBP_CLUT_SIZE
>  DVDVIDEO_SUBP_CLUT_LEN * sizeof(uint32_t)
> > +
> > +/* convert binary-coded decimal to decimal */
> > +#define BCD2D(__x__) (((__x__ & 0xF0) >> 4) * 10 + (__x__ & 0x0F))
> > +
> > +/* crop table for YUV to RGB subpicture palette conversion */
> > +#define DVDVIDEO_YUV_NEG_CROP_MAX                       1024
> > +#define times4(x) x, x, x, x
> > +#define times256(x) times4(times4(times4(times4(times4(x)))))
> > +
> > +const uint8_t dvdvideo_yuv_crop_tab[256 + 2 *
> DVDVIDEO_YUV_NEG_CROP_MAX] = {
> > +times256(0x00),
> >
> +0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0A,0x0B,0x0C,0x0D,0x0E,0x0F,
> >
> +0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,0x18,0x19,0x1A,0x1B,0x1C,0x1D,0x1E,0x1F,
> >
> +0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28,0x29,0x2A,0x2B,0x2C,0x2D,0x2E,0x2F,
> >
> +0x30,0x31,0x32,0x33,0x34,0x35,0x36,0x37,0x38,0x39,0x3A,0x3B,0x3C,0x3D,0x3E,0x3F,
> >
> +0x40,0x41,0x42,0x43,0x44,0x45,0x46,0x47,0x48,0x49,0x4A,0x4B,0x4C,0x4D,0x4E,0x4F,
> >
> +0x50,0x51,0x52,0x53,0x54,0x55,0x56,0x57,0x58,0x59,0x5A,0x5B,0x5C,0x5D,0x5E,0x5F,
> >
> +0x60,0x61,0x62,0x63,0x64,0x65,0x66,0x67,0x68,0x69,0x6A,0x6B,0x6C,0x6D,0x6E,0x6F,
> >
> +0x70,0x71,0x72,0x73,0x74,0x75,0x76,0x77,0x78,0x79,0x7A,0x7B,0x7C,0x7D,0x7E,0x7F,
> >
> +0x80,0x81,0x82,0x83,0x84,0x85,0x86,0x87,0x88,0x89,0x8A,0x8B,0x8C,0x8D,0x8E,0x8F,
> >
> +0x90,0x91,0x92,0x93,0x94,0x95,0x96,0x97,0x98,0x99,0x9A,0x9B,0x9C,0x9D,0x9E,0x9F,
> >
> +0xA0,0xA1,0xA2,0xA3,0xA4,0xA5,0xA6,0xA7,0xA8,0xA9,0xAA,0xAB,0xAC,0xAD,0xAE,0xAF,
> >
> +0xB0,0xB1,0xB2,0xB3,0xB4,0xB5,0xB6,0xB7,0xB8,0xB9,0xBA,0xBB,0xBC,0xBD,0xBE,0xBF,
> >
> +0xC0,0xC1,0xC2,0xC3,0xC4,0xC5,0xC6,0xC7,0xC8,0xC9,0xCA,0xCB,0xCC,0xCD,0xCE,0xCF,
> >
> +0xD0,0xD1,0xD2,0xD3,0xD4,0xD5,0xD6,0xD7,0xD8,0xD9,0xDA,0xDB,0xDC,0xDD,0xDE,0xDF,
> >
> +0xE0,0xE1,0xE2,0xE3,0xE4,0xE5,0xE6,0xE7,0xE8,0xE9,0xEA,0xEB,0xEC,0xED,0xEE,0xEF,
> >
> +0xF0,0xF1,0xF2,0xF3,0xF4,0xF5,0xF6,0xF7,0xF8,0xF9,0xFA,0xFB,0xFC,0xFD,0xFE,0xFF,
> > +times256(0xFF)
> > +};
> > +
> > +enum DVDVideoVTSMPEGVersion {
> > +    DVDVIDEO_VTS_MPEG_VERSION_MPEG1          = 0,
> > +    DVDVIDEO_VTS_MPEG_VERSION_MPEG2          = 1
> > +};
> > +
> > +enum DVDVideoVTSPictureFormat {
> > +    DVDVIDEO_VTS_PICTURE_FORMAT_NTSC         = 0,
> > +    DVDVIDEO_VTS_PICTURE_FORMAT_PAL          = 1
> > +};
> > +
> > +enum DVDVideoVTSPictureSize {
> > +    DVDVIDEO_VTS_PICTURE_SIZE_D1             = 0,
> > +    DVDVIDEO_VTS_PICTURE_SIZE_4CIF           = 1,
> > +    DVDVIDEO_VTS_PICTURE_SIZE_D1_HALF        = 2,
> > +    DVDVIDEO_VTS_PICTURE_SIZE_CIF            = 3
> > +};
> > +
> > +enum DVDVideoVTSPictureDAR {
> > +    DVDVIDEO_VTS_PICTURE_DAR_4_3             = 0,
> > +    DVDVIDEO_VTS_PICTURE_DAR_16_9            = 3
> > +};
> > +
> > +enum DVDVideoVTSPermittedFullscreenDisplay {
> > +    DVDVIDEO_VTS_SPU_PFD_ANY                 = 0,
> > +    DVDVIDEO_VTS_SPU_PFD_PANSCAN_ONLY        = 1,
> > +    DVDVIDEO_VTS_SPU_PFD_LETTERBOX_ONLY      = 2
> > +};
> > +
> > +enum DVDVideoVTSAudioFormat {
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_AC3            = 0,
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_MP1            = 2,
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_MP2            = 3,
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_PCM            = 4,
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_SDDS           = 5,
> > +    DVDVIDEO_VTS_AUDIO_FORMAT_DTS            = 6
> > +};
> > +
> > +enum FFDVDVideoVTSAudioSampleRate {
> > +    DVDVIDEO_VTS_AUDIO_SAMPLE_RATE_48K       = 0,
> > +    DVDVIDEO_VTS_AUDIO_SAMPLE_RATE_96K       = 1
> > +};
> > +
> > +enum FFDVDVideoVTSAudioQuantization {
> > +    DVDVIDEO_VTS_AUDIO_QUANTIZATION_16       = 0,
> > +    DVDVIDEO_VTS_AUDIO_QUANTIZATION_20       = 1,
> > +    DVDVIDEO_VTS_AUDIO_QUANTIZATION_24       = 2
> > +};
> > +
> > +typedef struct DVDVideoDemuxContext {
> > +    const AVClass               *class;
> > +
> > +    /* options */
> > +    int                         opt_title;                  /* the
> user-provided title number (1-indexed) */
> > +    int                         opt_ptt;                    /* the
> user-provided PTT number (1-indexed) */
> > +    int                         opt_pgc;                    /* the
> user-provided PGC number (1-indexed) */
> > +    int                         opt_pg;                     /* the
> user-provided PG number (1-indexed) */
> > +    int                         opt_angle;                  /* the
> user-provided angle number (1-indexed) */
> > +    int                         opt_region;                 /* the
> user-provided region identification digit */
> > +
> > +    /* subdemux */
> > +    const AVInputFormat         *mpeg_fmt;                  /* inner
> MPEG-PS (VOB) demuxer */
> > +    AVFormatContext             *mpeg_ctx;                  /* context
> for inner demuxer */
> > +    uint8_t                     *mpeg_buf;                  /* buffer
> for inner demuxer */
> > +    FFIOContext                 mpeg_pb;                    /* buffer
> context for inner demuxer */
> > +
> > +    /* volume */
> > +    dvd_reader_t                *vol_dvdread;               /* handle
> to libdvdread */
> > +    ifo_handle_t                *vol_vmg_ifo;               /* handle
> to the VMG (VIDEO_TS.IFO) */
> > +
> > +    /* playback */
> > +    ifo_handle_t                *play_vts_ifo;              /* handle
> to the active VTS (VTS_nn_n.IFO) */
> > +    pgc_t                       *play_pgc;                  /* handle
> to the active PGC */
> > +    int                         play_vtsn;                  /* number
> of the active VTS (video title set) */
> > +    int                         play_pgcn;                  /* number
> of the active PGC (program chain) */
> > +    dvdnav_t                    *play_dvdnav;               /* handle
> to libdvdnav */
> > +} DVDVideoDemuxContext;
> > +
> > +static void dvdvideo_libdvdread_log(void *opaque, dvd_logger_level_t
> level,
> > +        const char *msg, va_list msg_va)
> > +{
>
> This function is unnecessary. If you really need to intercept logged
> messages from libdvdnav and feed them to av_log, you should just have a
> lookup that maps the corresponding message level.
>
> > +    AVFormatContext *s = opaque;
> > +    int lavu_level;
> > +
> > +    char msg_buf[DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE] = {0};
> > +    vsnprintf(msg_buf, DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE, msg, msg_va);
> > +
> > +    switch (level) {
> > +        case DVD_LOGGER_LEVEL_ERROR:
> > +            lavu_level = AV_LOG_ERROR;
> > +            break;
> > +        case DVD_LOGGER_LEVEL_WARN:
> > +            lavu_level = AV_LOG_WARNING;
> > +            break;
> > +        case DVD_LOGGER_LEVEL_INFO:
> > +            lavu_level = AV_LOG_INFO;
> > +            break;
> > +        case DVD_LOGGER_LEVEL_DEBUG:
> > +        default:
> > +            lavu_level = AV_LOG_DEBUG;
> > +    }
> > +
> > +    /* libdvdread messages don't have line terminators */
> > +    av_log(s, lavu_level, "libdvdread: %s\n", msg_buf);
> > +}
> > +
> > +static void dvdvideo_libdvdnav_log(void *opaque, dvdnav_logger_level_t
> level,
> > +        const char *msg, va_list msg_va)
> > +{
> > +    AVFormatContext *s = opaque;
> Same complaint here. Also you have no to reassign opaque here.
>
> > +    int lavu_level;
> > +
> > +    char msg_buf[DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE] = {0};
> > +    vsnprintf(msg_buf, DVDVIDEO_LIBDVDX_LOG_BUFFER_SIZE, msg, msg_va);
> > +
> > +    switch (level) {
> > +        case DVDNAV_LOGGER_LEVEL_ERROR:
> > +            lavu_level = AV_LOG_ERROR;
> > +            break;
> > +        case DVDNAV_LOGGER_LEVEL_WARN:
> > +            lavu_level = AV_LOG_WARNING;
> > +            break;
> > +        case DVDNAV_LOGGER_LEVEL_INFO:
> > +            lavu_level = AV_LOG_INFO;
> > +            break;
> > +        case DVDNAV_LOGGER_LEVEL_DEBUG:
> > +        default:
> > +            lavu_level = AV_LOG_DEBUG;
> > +    }
> > +
> > +    /* libdvdnav messages don't have line terminators */
> > +    av_log(s, lavu_level, "libdvdnav: %s\n", msg_buf);
> > +}
> > +
> > +static char *dvdvideo_lang_code_to_iso639__2(const uint16_t lang_code)
> > +{
> > +    char lang_code_str[3] = {0};
> > +
> > +    if (lang_code && lang_code != 0xFFFF) {
> > +        lang_code_str[0] = (lang_code >> 8) & 0xFF;
> > +        lang_code_str[1] = lang_code & 0xFF;
> > +    }
> AV_WB16(lang_code_str, lang_code);
>
> > +
> > +    return (char *) ff_convert_lang_to(lang_code_str,
> AV_LANG_ISO639_2_BIBL);
> > +}
> > +
> > +static int64_t dvdvideo_time_to_millis(dvd_time_t *time)
> > +{
> > +    double fps;
> > +    int64_t ms;
> > +
> > +    int64_t sec = (int64_t) (BCD2D(time->hour)) * 60 * 60;
> > +    sec += (int64_t) (BCD2D(time->minute)) * 60;
> > +    sec += (int64_t) (BCD2D(time->second));
> > +
> > +    /* the 2 high bits are the frame rate */
> > +    switch ((time->frame_u & 0xC0) >> 6)
> > +    {
> > +        case 1:
> > +            fps = 25.0;
> > +            break;
> > +        case 3:
> > +            fps = 29.97;
> This is wrong, it's 30000/1001. Which you knew, because you #defined it
> above. Why not use those?
>
> > +            break;
> > +        default:
> > +            fps = 2500.0;
> > +            break;
> > +    }
> > +    ms = BCD2D(time->frame_u & 0x3F) * 1000.0 / fps;
> > +
> > +    return (sec * 1000) + ms;
> > +}
> > +
> > +static int dvdvideo_volume_is_title_valid_in_context(AVFormatContext *s,
> > +        int title)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    return title >= 1
> > +            && title <= DVDVIDEO_TITLE_NUMBER_MAX
> > +            && title <= c->vol_vmg_ifo->tt_srpt->nr_of_srpts
> > +            && c->vol_vmg_ifo->tt_srpt->title[title - 1].nr_of_ptts > 0;
> > +}
> > +
> > +static int dvdvideo_volume_is_title_valid_in_vts(title_info_t
> title_info,
> > +        ifo_handle_t *vts_ifo)
> > +{
> > +    return title_info.vts_ttn >= 1
> > +            && title_info.vts_ttn <= DVDVIDEO_TT_NUMBER_MAX
> > +            && title_info.vts_ttn <= vts_ifo->vts_ptt_srpt->nr_of_srpts
> > +            && vts_ifo->vtsi_mat->nr_of_vts_audio_streams
> > +                    <= DVDVIDEO_VTS_AUDIO_STREAMS_MAX
> > +            && vts_ifo->vtsi_mat->nr_of_vts_subp_streams
> > +                    <= DVDVIDEO_VTS_SUBP_STREAMS_MAX;
> > +}
> > +
> > +static int dvdvideo_volume_is_angle_valid_in_title(int angle,
> > +        title_info_t title_info)
> > +{
> > +    return angle >= 1
> > +            && angle <= DVDVIDEO_ANGLE_NUMBER_MAX
> > +            && angle <= title_info.nr_of_angles;
> > +}
> > +
> > +static int dvdvideo_volume_is_pgc_valid_and_sequential(pgc_t *pgc)
> > +{
> > +    return pgc
> > +            && pgc->program_map
> > +            && pgc->cell_playback != NULL
> > +            && pgc->pg_playback_mode == 0
> > +            && pgc->nr_of_programs > 0
> > +            && pgc->nr_of_cells > 0;
> > +}
> > +
> > +static int dvdvideo_volume_is_pgcn_in_vts(int pgcn, ifo_handle_t
> *vts_ifo)
> > +{
> > +    return pgcn >= 1
> > +            && pgcn <= DVDVIDEO_PGC_NUMBER_MAX
> > +            && pgcn <= vts_ifo->vts_pgcit->nr_of_pgci_srp;
> > +}
> > +
> > +static int dvdvideo_volume_is_pgn_in_pgc(int pgn, pgc_t *pgc)
> > +{
> > +    return pgn >= 1
> > +            && pgn <= DVDVIDEO_PG_NUMBER_MAX
> > +            && pgn <= pgc->nr_of_programs;
> > +}
> > +
> > +static int dvdvideo_volume_open(AVFormatContext *s)
> av_cold
>
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    dvd_reader_t *dvdread;
> > +    ifo_handle_t *vmg_ifo;
> > +
> > +    dvd_logger_cb dvdread_log_cb = { .pf_log = dvdvideo_libdvdread_log
> };
> > +    dvdread = DVDOpen2(s, &dvdread_log_cb, s->url);
> > +
> > +    if (!dvdread) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to initialize libdvdread\n");
> > +
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> > +    if (!(vmg_ifo = ifoOpen(dvdread, 0))) {
> > +        DVDClose(dvdread);
> > +
> > +        av_log(s, AV_LOG_ERROR,
> > +            "Unable to open VIDEO_TS.IFO. "
> > +            "Input does not have a valid DVD-Video structure "
> > +            "or is not a compliant UDF DVD-Video image.\n");
> > +
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    c->vol_dvdread = dvdread;
> > +    c->vol_vmg_ifo = vmg_ifo;
> > +
> > +    return 0;
> > +}
> > +
> > +static void dvdvideo_volume_close(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    if (c->vol_vmg_ifo) {
> > +        ifoClose(c->vol_vmg_ifo);
> Can this fail?
>
> > +        c->vol_vmg_ifo = NULL;
> > +    }
> > +
> > +    if (c->vol_dvdread) {
> > +        DVDClose(c->vol_dvdread);
> Can this fail?
>
> > +        c->vol_dvdread = NULL;
> > +    }
> > +}
> > +
> > +static int dvdvideo_volume_open_vts_ifo(AVFormatContext *s, int vtsn,
> > +        ifo_handle_t **p_vts_ifo)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    ifo_handle_t *vts_ifo;
> > +
> > +    if (vtsn < 1
> > +            || vtsn > DVDVIDEO_VTS_NUMBER_MAX
> > +            || !(vts_ifo = ifoOpen(c->vol_dvdread, vtsn))) {
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    for (int i = 0; i < vts_ifo->vts_c_adt->nr_of_vobs; i++) {
> > +        int start_sector =
> vts_ifo->vts_c_adt->cell_adr_table[i].start_sector;
> > +        int end_sector =
> vts_ifo->vts_c_adt->cell_adr_table[i].last_sector;
> > +
> > +        if ((start_sector & 0xFFFFFF) == 0xFFFFFF
> > +                || (end_sector & 0xFFFFFF) == 0xFFFFFF
> > +                || start_sector >= end_sector) {
> > +            ifoClose(vts_ifo);
> > +
> > +            av_log(s, AV_LOG_WARNING, "VTS has invalid cell address
> table\n");
> > +
> > +            return AVERROR_INVALIDDATA;
> > +        }
> > +    }
> > +
> > +    (*p_vts_ifo) = vts_ifo;
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_playback_open(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    int ret;
> > +
> > +    ifo_handle_t *vts_ifo;
> > +    dvdnav_logger_cb dvdnav_log_cb;
> > +    dvdnav_status_t dvdnav_status;
> > +    dvdnav_t *dvdnav;
> > +
> > +    title_info_t title_info;
> > +    int pgcn;
> > +    int pgn;
> > +    pgc_t *pgc;
> > +
> > +    int32_t disc_region_mask;
> > +    int32_t player_region_mask;
> > +    int cell_search_has_vts = 0;
> > +
> > +    /* if the title is valid, open its VTS IFO */
> > +    if (!dvdvideo_volume_is_title_valid_in_context(s, c->opt_title)) {
> > +        av_log(s, AV_LOG_ERROR, "Title not found or invalid\n");
> > +
> > +        return AVERROR_STREAM_NOT_FOUND;
> > +    }
> > +
> > +    title_info = c->vol_vmg_ifo->tt_srpt->title[c->opt_title - 1];
> > +
> > +    if ((ret = dvdvideo_volume_open_vts_ifo(s,
> > +            title_info.title_set_nr, &vts_ifo)) != 0) {
> Our convention is to return negative values on error in FFmpeg. Not
> nonzero values.
>
> > +        av_log(s, AV_LOG_ERROR, "Title VTS could not be opened\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    if (!dvdvideo_volume_is_title_valid_in_vts(title_info, vts_ifo)) {
> > +        av_log(s, AV_LOG_ERROR, "Title VTS is invalid\n");
> > +
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    if (!dvdvideo_volume_is_angle_valid_in_title(c->opt_angle,
> title_info)) {
> > +        av_log(s, AV_LOG_ERROR, "Angle not found or invalid\n");
> > +
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    /* determine our PGC and PG (playback coordinates) */
> > +    if (c->opt_pgc > 0 && c->opt_pg > 0) {
> > +        if (c->opt_ptt > 0) {
> > +            av_log(s, AV_LOG_WARNING,
> > +                    "PTT option ignored as PGC and PG are provided\n");
> > +        }
> > +
> > +        pgcn = c->opt_pgc;
> > +        pgn = c->opt_pg;
> > +    } else {
> > +        if (c->opt_ptt > title_info.nr_of_ptts) {
> > +            av_log(s, AV_LOG_ERROR, "PTT not found\n");
> > +
> > +            return AVERROR_STREAM_NOT_FOUND;
> > +        }
> > +
> > +        pgcn = vts_ifo->vts_ptt_srpt->title[title_info.vts_ttn -
> 1].ptt[c->opt_ptt - 1].pgcn;
> > +        pgn = vts_ifo->vts_ptt_srpt->title[title_info.vts_ttn -
> 1].ptt[c->opt_ptt - 1].pgn;
> > +    }
> > +
> > +    if (!dvdvideo_volume_is_pgcn_in_vts(pgcn, vts_ifo)) {
> > +        av_log(s, AV_LOG_ERROR, "PGC not found\n");
> > +
> > +        return AVERROR_STREAM_NOT_FOUND;
> > +    }
> > +
> > +    pgc = vts_ifo->vts_pgcit->pgci_srp[pgcn - 1].pgc;
> > +
> > +    if (!dvdvideo_volume_is_pgc_valid_and_sequential(pgc)) {
> > +        av_log(s, AV_LOG_ERROR, "PGC not valid\n");
> > +
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    if (!dvdvideo_volume_is_pgn_in_pgc(pgn, pgc)) {
> > +        av_log(s, AV_LOG_ERROR, "PG not found\n");
> > +
> > +        return AVERROR_STREAM_NOT_FOUND;
> > +    }
> > +
> > +    /* set up libdvdnav */
> > +    dvdnav_log_cb = (dvdnav_logger_cb) { .pf_log =
> dvdvideo_libdvdnav_log };
> > +    dvdnav_status = dvdnav_open2(&dvdnav, NULL, &dvdnav_log_cb, s->url);
> > +
> > +    if (!dvdnav) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to initialize libdvdnav\n");
> > +
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> > +    if (dvdnav_status != DVDNAV_STATUS_OK) {
> Code style. We don't use braces with single-line blocks.
>
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (dvdnav_set_readahead_flag(dvdnav, 0) != DVDNAV_STATUS_OK) {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (dvdnav_set_PGC_positioning_flag(dvdnav, 1) != DVDNAV_STATUS_OK)
> {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (dvdnav_get_region_mask(dvdnav, &disc_region_mask) !=
> DVDNAV_STATUS_OK) {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (c->opt_region > 0) {
> > +        player_region_mask = (1 << (c->opt_region - 1));
> > +    } else {
> > +        player_region_mask = disc_region_mask;
> > +    }
> > +
> > +    if (dvdnav_set_region_mask(dvdnav, player_region_mask) !=
> DVDNAV_STATUS_OK) {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (dvdnav_program_play(dvdnav, c->opt_title, pgcn, pgn) !=
> DVDNAV_STATUS_OK) {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    if (dvdnav_angle_change(dvdnav, c->opt_angle) != DVDNAV_STATUS_OK) {
> > +        goto end_search_error_dvdnav;
> > +    }
> > +
> > +    /* lock on to title's VTS and chosen PGC/PGN coordinates */
> > +    for (int i = 0; i < DVDVIDEO_PS_MAX_SEARCH_BLOCKS; i++) {
> > +        int nav_event;
> > +        int nav_len;
> > +        dvdnav_vts_change_event_t *vts_event;
> > +        uint8_t nav_buf[DVDVIDEO_BLOCK_SIZE] = {0};
> > +
> > +        if (ff_check_interrupt(&s->interrupt_callback)) {
> > +            return AVERROR_EXIT;
> > +        }
> > +
> > +        if (dvdnav_get_next_block(dvdnav, nav_buf, &nav_event, &nav_len)
> > +                != DVDNAV_STATUS_OK) {
> > +            goto end_search_error_dvdnav;
> > +        }
> > +
> > +        if (nav_len > DVDVIDEO_BLOCK_SIZE) {
> > +            av_log(s, AV_LOG_ERROR, "Invalid block (too big)\n");
> > +
> > +            ret = AVERROR(ENOMEM);
> > +
> > +            goto end_search_error;
> > +        }
> > +
> > +        av_log(s, AV_LOG_TRACE, "nav event=%d len=%d\n", nav_event,
> nav_len);
> > +
> > +        switch (nav_event) {
> > +            case DVDNAV_VTS_CHANGE:
> > +                vts_event = (dvdnav_vts_change_event_t *) nav_buf;
> > +
> > +                if (cell_search_has_vts) {
> > +                    if (vts_event->new_vtsN != title_info.title_set_nr
> > +                            || vts_event->new_domain !=
> DVD_DOMAIN_VTSTitle) {
> > +                        av_log(s, AV_LOG_ERROR, "Unexpected VTS
> change\n");
> > +
> > +                        ret = AVERROR_INPUT_CHANGED;
> > +
> > +                        goto end_search_error;
> > +                    }
> > +                    continue;
> > +                }
> > +
> > +                if (vts_event->new_vtsN == title_info.title_set_nr
> > +                        && vts_event->new_domain ==
> DVD_DOMAIN_VTSTitle) {
> > +                    cell_search_has_vts = 1;
> > +                }
> > +
> > +                continue;
> > +            case DVDNAV_CELL_CHANGE:
> > +                // if we need more info about the cell:
> > +                // dvdnav_cell_change_event_t *event_data =
> > +                //     (dvdnav_cell_change_event_t *) nav_buf;
> > +                int check_title;
> > +                int check_pgcn;
> > +                int check_pgn;
> > +
> > +                if (!cell_search_has_vts) {
> > +                    continue;
> > +                }
> > +
> > +                dvdnav_current_title_program(dvdnav, &check_title,
> > +                                          &check_pgcn, &check_pgn);
> > +
> > +                if (check_title == c->opt_title && check_pgcn == pgcn
> > +                        && check_pgn == pgn &&
> dvdnav_is_domain_vts(dvdnav)) {
> > +                    goto end_ready;
> > +                }
> > +
> > +                continue;
> > +            case DVDNAV_STILL_FRAME:
> > +                dvdnav_still_skip(dvdnav);
> > +
> > +                continue;
> > +            case DVDNAV_WAIT:
> > +                dvdnav_wait_skip(dvdnav);
> > +
> > +                continue;
> > +            case DVDNAV_STOP:
> > +                ret = AVERROR_INPUT_CHANGED;
> > +
> > +                goto end_search_error;
> > +            default:
> > +                continue;
> > +        }
> > +    }
> > +
> > +end_search_error_dvdnav:
> > +    ret = AVERROR_EXTERNAL;
> > +    av_log(s, AV_LOG_ERROR, "libdvdnav: %s\n",
> dvdnav_err_to_string(dvdnav));
> > +    av_log(s, AV_LOG_ERROR, "Error starting playback\n");
> > +
> > +end_search_error:
> > +    dvdnav_close(dvdnav);
> > +    ifoClose(vts_ifo);
> > +
> > +    return ret;
> > +
> > +end_ready:
> > +    /* update the context */
> > +    c->play_vts_ifo = vts_ifo;
> > +    c->play_pgc = pgc;
> > +    c->play_vtsn = title_info.title_set_nr;
> > +    c->play_pgcn = pgcn;
> > +    c->play_dvdnav = dvdnav;
> > +
> > +    return 0;
> > +}
> > +
> > +static void dvdvideo_playback_close(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    if (c->play_dvdnav) {
> > +        dvdnav_close(c->play_dvdnav);
> > +        c->play_dvdnav = NULL;
> > +    }
> > +
> > +    if (c->play_vts_ifo) {
> > +        ifoClose(c->play_vts_ifo);
> > +        c->play_vts_ifo = NULL;
> > +    }
> > +}
> > +
> > +static int dvdvideo_video_stream_analyze(video_attr_t video_attr,
> > +        enum AVCodecID *p_codec_id, AVRational *p_framerate,
> > +        int *p_height, int *p_width)
> > +{
> > +    enum AVCodecID codec_id = AV_CODEC_ID_NONE;
> > +    AVRational framerate = ZERO_Q;
> > +    int height = 0;
> > +    int width = 0;
> > +
> > +    switch (video_attr.mpeg_version) {
> > +        case DVDVIDEO_VTS_MPEG_VERSION_MPEG1:
> > +            codec_id = AV_CODEC_ID_MPEG1VIDEO;
> > +            break;
> > +        case DVDVIDEO_VTS_MPEG_VERSION_MPEG2:
> > +            codec_id = AV_CODEC_ID_MPEG2VIDEO;
> > +            break;
> > +    }
> > +
> > +    switch (video_attr.video_format) {
> > +        case DVDVIDEO_VTS_PICTURE_FORMAT_NTSC:
> > +            framerate = DVDVIDEO_NTSC_FRAMERATE_Q;
> > +            height = DVDVIDEO_NTSC_HEIGHT;
> > +            break;
> > +        case DVDVIDEO_VTS_PICTURE_FORMAT_PAL:
> > +            framerate = DVDVIDEO_PAL_FRAMERATE_Q;
> > +            height = DVDVIDEO_PAL_HEIGHT;
> > +            break;
> > +    }
> > +
> > +    if (height > 0) {
> > +        switch (video_attr.picture_size) {
> > +            case DVDVIDEO_VTS_PICTURE_SIZE_D1:
> > +                width = DVDVIDEO_D1_WIDTH;
> > +                break;
> > +            case DVDVIDEO_VTS_PICTURE_SIZE_4CIF:
> > +                width = DVDVIDEO_4CIF_WIDTH;
> > +                break;
> > +            case DVDVIDEO_VTS_PICTURE_SIZE_D1_HALF:
> > +                width = DVDVIDEO_D1_HALF_WIDTH;
> > +                break;
> > +            case DVDVIDEO_VTS_PICTURE_SIZE_CIF:
> > +                width = DVDVIDEO_CIF_WIDTH;
> > +                height /= 2;
> > +                break;
> > +        }
> > +    }
> > +
> > +    if (codec_id == AV_CODEC_ID_NONE || av_cmp_q(framerate, ZERO_Q) == 0
> av_cmp_q for comparing a rational to zero is unnecessary. Just check if
> the numerator is nonzero, as a fraction is zero if and only if its
> numerator is zero.
>
> > +            || width < 1 || height < 1) {
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    (*p_codec_id) = codec_id;
> > +    (*p_framerate) = framerate;
> > +    (*p_height) = height;
> > +    (*p_width) = width;
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_video_stream_to_avstream(AVFormatContext *s,
> > +        enum AVCodecID codec_id, AVRational framerate,
> > +        int height, int width,
> > +        enum AVStreamParseType need_parsing)
> > +{
> > +    AVStream *st;
> > +    FFStream *sti;
> > +
> > +    st = avformat_new_stream(s, NULL);
> > +    if (!st) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> > +    st->id = DVDVIDEO_STARTCODE_VIDEO;
> > +    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> > +    st->codecpar->codec_id = codec_id;
> > +    st->codecpar->width = width;
> > +    st->codecpar->height = height;
> > +    st->codecpar->format = DVDVIDEO_PIXEL_FORMAT;
> > +
> > +    st->codecpar->framerate = framerate;
> > +#if FF_API_R_FRAME_RATE
> > +    st->r_frame_rate = framerate;
> > +#endif
> > +    st->avg_frame_rate = framerate;
> > +
> > +    avpriv_set_pts_info(st, DVDVIDEO_PTS_WRAP_BITS,
> > +        DVDVIDEO_TIME_BASE_Q.num, DVDVIDEO_TIME_BASE_Q.den);
> > +
> > +    sti = ffstream(st);
> > +    sti->request_probe = 0;
> > +    sti->need_parsing = need_parsing;
> > +    sti->avctx->framerate = framerate;
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_video_stream_register(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    enum AVCodecID codec_id;
> > +    AVRational framerate;
> > +    int height;
> > +    int width;
> > +
> > +    int ret;
> > +
> > +    if ((ret =
> dvdvideo_video_stream_analyze(c->play_vts_ifo->vtsi_mat->vts_video_attr,
> &codec_id,
> > +            &framerate, &height, &width)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Invalid video parameters in VTS
> IFO\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_video_stream_to_avstream(c->mpeg_ctx, codec_id,
> > +            framerate, height, width, AVSTREAM_PARSE_FULL)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to allocate video stream
> (subdemux)\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_video_stream_to_avstream(s, codec_id,
> > +            framerate, height, width, AVSTREAM_PARSE_NONE)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to allocate video stream\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_audio_stream_analyze(audio_attr_t audio_attr,
> > +        uint16_t audio_control, enum AVCodecID *p_codec_id, int
> *p_startcode,
> > +        int *p_sample_rate, int *p_sample_bits,
> > +        int *p_nb_channels, char **lang_iso639__2)
> > +{
> > +    enum AVCodecID codec_id = AV_CODEC_ID_NONE;
> > +    int startcode = 0;
> > +    int sample_rate = 0;
> > +    int sample_bits = 0;
> > +    int nb_channels = 0;
> > +
> > +    int position = (audio_control & 0x7F00) >> 8;
> > +
> > +    switch (audio_attr.audio_format) {
> > +        case DVDVIDEO_VTS_AUDIO_FORMAT_AC3:
> > +            codec_id = AV_CODEC_ID_AC3;
> > +            sample_rate = 48000;
> > +            startcode = DVDVIDEO_STARTCODE_OFFSET_AUDIO_AC3 + position;
> > +            break;
> > +        case DVDVIDEO_VTS_AUDIO_FORMAT_MP1:
> > +            codec_id = AV_CODEC_ID_MP1;
> > +            sample_rate = 48000;
> > +            startcode = DVDVIDEO_STARTCODE_OFFSET_AUDIO_MP1 + position;
> > +            break;
> > +        case DVDVIDEO_VTS_AUDIO_FORMAT_MP2:
> > +            codec_id = AV_CODEC_ID_MP2;
> > +            sample_rate = 48000;
> > +            startcode = DVDVIDEO_STARTCODE_OFFSET_AUDIO_MP2 + position;
> > +            break;
> > +        case DVDVIDEO_VTS_AUDIO_FORMAT_PCM:
> > +            codec_id = AV_CODEC_ID_PCM_DVD;
> > +
> > +            switch (audio_attr.sample_frequency) {
> > +                case DVDVIDEO_VTS_AUDIO_SAMPLE_RATE_48K:
> > +                    sample_rate = 48000;
> > +                    break;
> > +                case DVDVIDEO_VTS_AUDIO_SAMPLE_RATE_96K:
> > +                    sample_rate = 96000;
> > +                    break;
> > +            }
> > +
> > +            switch (audio_attr.quantization) {
> > +                case DVDVIDEO_VTS_AUDIO_QUANTIZATION_16:
> > +                    sample_bits = 16;
> > +                    break;
> > +                case DVDVIDEO_VTS_AUDIO_QUANTIZATION_20:
> > +                    sample_bits = 20;
> > +                    break;
> > +                case DVDVIDEO_VTS_AUDIO_QUANTIZATION_24:
> > +                    sample_bits = 24;
> > +                    break;
> > +            }
> > +
> > +            startcode = DVDVIDEO_STARTCODE_OFFSET_AUDIO_PCM + position;
> > +            break;
> > +        case DVDVIDEO_VTS_AUDIO_FORMAT_DTS:
> > +            codec_id = AV_CODEC_ID_DTS;
> > +            sample_rate = 48000;
> > +            startcode = DVDVIDEO_STARTCODE_OFFSET_AUDIO_DTS + position;
> > +            break;
> > +    }
> > +
> > +    nb_channels = audio_attr.channels + 1;
> > +
> > +    if (codec_id == AV_CODEC_ID_NONE || startcode == 0
> > +            || sample_rate == 0 || nb_channels == 0) {
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    (*p_codec_id) = codec_id;
> > +    (*p_startcode) = startcode;
> > +    (*p_sample_rate) = sample_rate;
> > +    (*p_sample_bits) = sample_bits;
> > +    (*p_nb_channels) = nb_channels;
> > +    (*lang_iso639__2) =
> dvdvideo_lang_code_to_iso639__2(audio_attr.lang_code);
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_audio_stream_to_avstream(AVFormatContext *s,
> > +        enum AVCodecID codec_id, int startcode, int sample_rate,
> > +        int sample_bits, int nb_channels, char *lang_iso639__2,
> > +        enum AVStreamParseType need_parsing)
> > +{
> > +    AVStream *st;
> > +    FFStream *sti;
> > +
> > +    st = avformat_new_stream(s, NULL);
> > +    if (!st) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> > +    st->id = startcode;
> > +    st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> > +    st->codecpar->codec_id = codec_id;
> > +    st->codecpar->sample_rate = sample_rate;
> > +    st->codecpar->ch_layout.nb_channels = nb_channels;
> > +
> > +    if (sample_bits > 0) {
> > +        st->codecpar->format = sample_bits == 16 ?
> > +                AV_SAMPLE_FMT_S16
> > +                : AV_SAMPLE_FMT_S32;
> > +        st->codecpar->bits_per_coded_sample = sample_bits;
> > +        st->codecpar->bits_per_raw_sample = sample_bits;
> > +    }
> > +
> > +    if (lang_iso639__2) {
> > +        av_dict_set(&st->metadata, "language", lang_iso639__2, 0);
> > +    }
> > +
> > +    avpriv_set_pts_info(st, DVDVIDEO_PTS_WRAP_BITS,
> > +        DVDVIDEO_TIME_BASE_Q.num, DVDVIDEO_TIME_BASE_Q.den);
> > +
> > +    sti = ffstream(st);
> > +    sti->request_probe = 0;
> > +    sti->need_parsing = need_parsing;
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_audio_stream_register_all(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    for (int i = 0; i <
> c->play_vts_ifo->vtsi_mat->nr_of_vts_audio_streams; i++) {
> > +        enum AVCodecID codec_id;
> > +        int startcode;
> > +        int sample_rate;
> > +        int sample_bits;
> > +        int nb_channels;
> > +        char *lang_iso639__2;
> > +        int ret;
> > +
> > +        if (!(c->play_pgc->audio_control[i]
> > +                & DVDVIDEO_AUDIO_CONTROL_ENABLED_MASK)) {
> > +            continue;
> > +        }
> > +
> > +        if ((ret = dvdvideo_audio_stream_analyze(
> > +                c->play_vts_ifo->vtsi_mat->vts_audio_attr[i],
> > +                c->play_pgc->audio_control[i], &codec_id, &startcode,
> > +                &sample_rate, &sample_bits, &nb_channels,
> > +                &lang_iso639__2)) != 0) {
> > +            av_log(s, AV_LOG_ERROR, "Invalid audio parameters in VTS
> IFO\n");
> > +
> > +            return ret;
> > +        }
> > +
> > +        if (c->play_vts_ifo->vtsi_mat->
> > +                vts_audio_attr[i].application_mode == 1) {
> > +            av_log(s, AV_LOG_ERROR,
> > +                "Audio stream uses karaoke extension which is
> unsupported\n");
> > +
> > +            return AVERROR_PATCHWELCOME;
> > +        }
> > +
> > +        for (int j = 0; j < s->nb_streams; j++) {
> > +            if (s->streams[j]->id == startcode) {
> > +                av_log(s, AV_LOG_WARNING,
> > +                        "Audio stream has duplicate entry in VTS
> IFO\n");
> > +
> > +                continue;
> > +            }
> > +        }
> > +
> > +        if ((ret = dvdvideo_audio_stream_to_avstream(c->mpeg_ctx,
> codec_id,
> > +                startcode, sample_rate, sample_bits, nb_channels,
> > +                lang_iso639__2, AVSTREAM_PARSE_FULL)) != 0) {
> > +            av_log(s, AV_LOG_ERROR, "Unable to allocate video stream
> (subdemux)\n");
> > +
> > +            return ret;
> > +        }
> > +
> > +        if ((ret = dvdvideo_audio_stream_to_avstream(s, codec_id,
> > +                startcode, sample_rate, sample_bits, nb_channels,
> > +                lang_iso639__2, AVSTREAM_PARSE_NONE)) != 0) {
> > +            av_log(s, AV_LOG_ERROR, "Unable to allocate audio
> stream\n");
> > +
> > +            return ret;
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subtitle_clut_rgb_extradata_cat(
> > +        const uint32_t *clut_rgb, size_t clut_rgb_size,
> > +        char *dst, size_t dst_size)
> > +{
> > +    if (dst_size < DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE) {
> > +        return AVERROR(ENOMEM);This is the error code for an allocation
> failure, which is not what is
> happening here. It's an illegal argument passed to the function.
>
> > +    }
> > +
> > +    if (clut_rgb_size != DVDVIDEO_SUBP_CLUT_SIZE) {
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> > +    snprintf(dst, dst_size, "palette: ");
> > +
> > +    for (int i = 0; i < DVDVIDEO_SUBP_CLUT_LEN; i++) {
> > +        av_strlcatf(dst, dst_size,
> > +                "%06"PRIx32"%s", clut_rgb[i], i != 15 ? ", " : "");
> > +    }
> > +
> > +    av_strlcat(dst, "\n", 1);
> > +
> You should be using the AVBPrint api here to construct a string, not
> av_strlcat.
>
>
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subtitle_clut_yuv_to_rgb(uint32_t *clut,
> > +        const size_t clut_size)
> What is a yuv-to-rgb function doing inside a dvd-video demuxer?
>
> > +{
> > +    const uint8_t *cm = dvdvideo_yuv_crop_tab +
> DVDVIDEO_YUV_NEG_CROP_MAX;
> > +
> > +    int i, y, cb, cr;
> > +    uint8_t r, g, b;
> > +    int r_add, g_add, b_add;
> > +
> > +    if (clut_size != DVDVIDEO_SUBP_CLUT_SIZE) {Why even have an
> argument to the function if it must equal some exact
> integer? At that point just remove clut_size from the function argument
> and just use DVDVIDEO_SUBP_CLUT_SIZE in its place.
>
>
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> > +    for (i = 0; i < DVDVIDEO_SUBP_CLUT_LEN; i++) {
> > +        y  = (clut[i] >> 16) & 0xFF;
> > +        cr = (clut[i] >> 8)  & 0xFF;
> > +        cb = clut[i]         & 0xFF;
> > +
> > +        YUV_TO_RGB1_CCIR(cb, cr);
> > +        YUV_TO_RGB2_CCIR(r, g, b, y);
> > +
> > +        clut[i] = (r << 16) | (g << 8) | b;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subtitle_stream_to_avstream(AVFormatContext *s,
> > +        int startcode,
> > +        char *clut_rgb_extradata_buf, int clut_rgb_extradata_buf_size,
> > +        char *lang_iso639__2, enum AVStreamParseType need_parsing)
> > +{
> > +    AVStream *st;
> > +    FFStream *sti;
> > +    int ret;
> > +
> > +    st = avformat_new_stream(s, NULL);
> > +    if (!st) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> > +    st->id = startcode;
> > +    st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
> > +    st->codecpar->codec_id = AV_CODEC_ID_DVD_SUBTITLE;
> > +
> > +    if ((ret = ff_alloc_extradata(st->codecpar,
> > +            clut_rgb_extradata_buf_size)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    memcpy(st->codecpar->extradata, clut_rgb_extradata_buf,
> > +            st->codecpar->extradata_size);
> > +
> > +    if (lang_iso639__2) {
> > +        av_dict_set(&st->metadata, "language", lang_iso639__2, 0);
> > +    }
> > +
> > +    avpriv_set_pts_info(st, DVDVIDEO_PTS_WRAP_BITS,
> > +        DVDVIDEO_TIME_BASE_Q.num, DVDVIDEO_TIME_BASE_Q.den);
> > +
> > +    sti = ffstream(st);
> > +    sti->request_probe = 0;
> > +    sti->need_parsing = need_parsing;
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subtitle_stream_register(AVFormatContext *s,
> > +        int position,
> > +        char *clut_rgb_extradata_buf, int clut_rgb_extradata_buf_size,
> > +        char *lang_iso639__2)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    int ret;
> > +
> > +    int startcode = DVDVIDEO_STARTCODE_OFFSET_SUBP + position;
> > +
> > +    for (int i = 0; i < s->nb_streams; i++) {
> > +        /* don't need to warn the user about this, since params won't
> change */
> > +        if (s->streams[i]->id == startcode) {
> > +            av_log(s, AV_LOG_TRACE,
> > +                    "Subtitle stream has duplicate entry in VTS IFO\n");
> > +            return 0;
> > +        }
> > +    }
> > +
> > +    if ((ret = dvdvideo_subtitle_stream_to_avstream(c->mpeg_ctx,
> startcode,
> > +            clut_rgb_extradata_buf, clut_rgb_extradata_buf_size,
> > +            lang_iso639__2, AVSTREAM_PARSE_FULL)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to allocate subtitle stream
> (subdemux)\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_subtitle_stream_to_avstream(s, startcode,
> > +            clut_rgb_extradata_buf, clut_rgb_extradata_buf_size,
> > +            lang_iso639__2, AVSTREAM_PARSE_NONE)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to allocate subtitle stream\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subtitle_stream_register_all(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    int ret;
> > +    uint32_t clut_rgb[DVDVIDEO_SUBP_CLUT_SIZE] = {0};
> > +    char clut_rgb_extradata_buf[DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE]
> = {0};
> > +
> > +    if (c->play_vts_ifo->vtsi_mat->nr_of_vts_subp_streams < 1) {
> > +        return 0;
> > +    }
> > +
> > +    /* initialize the palette (same for all streams in this PGC) */
> > +    memcpy(clut_rgb, c->play_pgc->palette, DVDVIDEO_SUBP_CLUT_SIZE);
> > +
> > +    if ((ret = dvdvideo_subtitle_clut_yuv_to_rgb(
> > +            clut_rgb, DVDVIDEO_SUBP_CLUT_SIZE)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to convert subtitle
> palette\n");Why are you converting the subtitle palette?
>
> > +
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_subtitle_clut_rgb_extradata_cat(
> > +            clut_rgb, DVDVIDEO_SUBP_CLUT_SIZE,
> > +            clut_rgb_extradata_buf,
> DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE)) != 0) {
> > +        av_log(s, AV_LOG_ERROR, "Unable to set up subtitle
> extradata\n");
> > +
> > +        return ret;
> > +    }
> > +
> > +    for (int i = 0; i < c->play_vts_ifo->
> > +            vtsi_mat->nr_of_vts_subp_streams; i++) {
> > +        uint32_t subp_control;
> > +        subp_attr_t subp_attr;
> > +        video_attr_t video_attr;
> > +        char *lang_iso639__2;
> > +
> > +        subp_control = c->play_pgc->subp_control[i];
> > +
> > +        if (!(subp_control & DVDVIDEO_SUBP_CONTROL_ENABLED_MASK)) {
> > +            continue;
> > +        }
> > +
> > +        subp_attr = c->play_vts_ifo->vtsi_mat->vts_subp_attr[i];
> > +        video_attr = c->play_vts_ifo->vtsi_mat->vts_video_attr;
> > +        lang_iso639__2 =
> dvdvideo_lang_code_to_iso639__2(subp_attr.lang_code);
> > +
> > +        /* there can be several presentations for one SPU */
> > +        /* for now, be flexible with the DAR check due to weird
> authoring */
> > +        if (video_attr.display_aspect_ratio > 0) {
> > +            /* 16:9 */
> > +            ret = dvdvideo_subtitle_stream_register(s,
> > +                ((subp_control >> 16) & 0x1F),
> > +                clut_rgb_extradata_buf,
> > +                DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE,
> > +                lang_iso639__2);
> > +            if (ret != 0) {
> > +                return ret;
> > +            }
> > +
> > +            if (video_attr.permitted_df ==
> DVDVIDEO_VTS_SPU_PFD_LETTERBOX_ONLY
> > +                    || video_attr.permitted_df ==
> DVDVIDEO_VTS_SPU_PFD_ANY) {
> > +                ret = dvdvideo_subtitle_stream_register(s,
> > +                    ((subp_control >> 8) & 0x1F),
> > +                    clut_rgb_extradata_buf,
> > +                    DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE,
> > +                    lang_iso639__2);
> > +                if (ret != 0) {
> > +                    return ret;
> > +                }
> > +            }
> > +
> > +            if (video_attr.permitted_df ==
> DVDVIDEO_VTS_SPU_PFD_PANSCAN_ONLY
> > +                    || video_attr.permitted_df ==
> DVDVIDEO_VTS_SPU_PFD_ANY) {
> > +                ret = dvdvideo_subtitle_stream_register(s,
> > +                    (subp_control & 0x1F),
> > +                    clut_rgb_extradata_buf,
> > +                    DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE,
> > +                    lang_iso639__2);
> > +                if (ret != 0) {
> > +                    return ret;
> > +                }
> > +            }
> > +        } else {
> > +            /* 4:3 */
> > +            ret = dvdvideo_subtitle_stream_register(s,
> > +                ((subp_control >> 24) & 0x1F),
> > +                clut_rgb_extradata_buf,
> > +                DVDVIDEO_SUBP_CLUT_RGB_EXTRADATA_SIZE,
> > +                lang_iso639__2);
> > +            if (ret != 0) {
> > +                return ret;
> > +            }
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_subdemux_nested_io_open(AVFormatContext *s,
> > +        AVIOContext **pb, const char *url, int flags, AVDictionary
> **opts)
> > +{
> This function is pointless.
>
> > +    av_log(s, AV_LOG_ERROR, "Nested io_open not supported for this
> format\n");
> > +
> > +    return AVERROR(EPERM);
> > +}
> > +
> > +static int dvdvideo_subdemux_read_data(void *opaque, uint8_t *buf,
> > +        int buf_size)
> > +{
> > +    AVFormatContext *s = opaque;
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    if (buf_size != DVDVIDEO_BLOCK_SIZE) {
> > +        av_log(s, AV_LOG_ERROR, "Invalid buffer size\n");
> > +
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> > +    for (int i = 0; i < DVDVIDEO_PS_MAX_SEARCH_BLOCKS; i++) {
> > +        int nav_event;
> > +        int nav_len;
> > +
> > +        int check_title;
> > +        int check_pgcn;
> > +        int check_pgn;
> > +        int check_angle;
> > +        int check_nb_angles;
> > +
> > +        uint8_t nav_buf[DVDVIDEO_BLOCK_SIZE] = {0};
> > +
> > +        if (ff_check_interrupt(&s->interrupt_callback)) {
> > +            return AVERROR_EXIT;
> > +        }
> > +
> > +        if (dvdnav_get_next_block(c->play_dvdnav,
> > +                nav_buf, &nav_event, &nav_len) != DVDNAV_STATUS_OK) {
> > +            av_log(s, AV_LOG_ERROR, "Error reading block\n");
> > +
> > +            goto end_dvdnav_error;
> > +        }
> > +
> > +        if (nav_len > DVDVIDEO_BLOCK_SIZE) {
> > +            av_log(s, AV_LOG_ERROR, "Invalid block (too big)\n");
> > +
> This is not what ENOMEM means.
>
> > +            return AVERROR(ENOMEM);
> > +        }
> > +
> > +        if (nav_event != DVDNAV_BLOCK_OK && nav_event !=
> DVDNAV_NAV_PACKET) {
> > +            av_log(s, AV_LOG_TRACE, "nav event=%d len=%d\n", nav_event,
> nav_len);
> > +        }
> > +
> > +        if (dvdnav_current_title_program(c->play_dvdnav, &check_title,
> > +                &check_pgcn, &check_pgn) != DVDNAV_STATUS_OK) {
> > +            goto end_dvdnav_error;
> > +        }
> > +
> > +        if (check_title != c->opt_title || check_pgcn != c->play_pgcn
> > +                || !dvdnav_is_domain_vts(c->play_dvdnav)) {
> > +            return AVERROR_EOF;
> > +        }
> > +
> > +        if (dvdnav_get_angle_info(c->play_dvdnav, &check_angle,
> > +                &check_nb_angles) != DVDNAV_STATUS_OK) {
> > +            goto end_dvdnav_error;
> > +        }
> > +
> > +        if (check_angle != c->opt_angle) {
> > +            av_log(s, AV_LOG_ERROR, "Unexpected angle change\n");
> > +
> > +            return AVERROR_INPUT_CHANGED;
> > +        }
> > +
> > +        switch (nav_event) {
> > +            case DVDNAV_BLOCK_OK:
> > +                if (nav_len != DVDVIDEO_BLOCK_SIZE) {
> > +                    av_log(s, AV_LOG_ERROR, "Invalid block\n");
> > +
> > +                    return AVERROR_INVALIDDATA;
> > +                }
> > +
> > +                memcpy(buf, &nav_buf, nav_len);
> > +
> > +                return nav_len;
> > +            case DVDNAV_HIGHLIGHT:
> > +            case DVDNAV_VTS_CHANGE:
> > +            case DVDNAV_STILL_FRAME:
> > +            case DVDNAV_STOP:
> > +            case DVDNAV_WAIT:
> > +                return AVERROR_EOF;
> > +            default:
> > +                continue;
> > +        }
> > +    }
> > +
> > +    av_log(s, AV_LOG_ERROR, "Unable to find next MPEG block\n");
> > +
> > +    return AVERROR_UNKNOWN;
> > +
> > +end_dvdnav_error:
> > +    av_log(s, AV_LOG_ERROR, "libdvdnav: %s\n",
> dvdnav_err_to_string(c->play_dvdnav));
> > +
> > +    return AVERROR_EXTERNAL;
> > +}
> > +
> > +static int64_t dvdvideo_subdemux_seek_data(void *opaque, int64_t offset,
> > +        int whence)
> > +{
> > +    AVFormatContext *s = opaque;
> > +
> > +    av_log(s, AV_LOG_ERROR,
> > +            "dvdvideo_subdemux_seek_data(): not implemented\n");
> > +
> > +    return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +static int dvdvideo_subdemux_open(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    int ret;
> > +
> > +    const AVInputFormat *mpeg_fmt;
> > +    AVFormatContext *mpeg_ctx;
> > +    uint8_t *mpeg_buf;
> > +
> > +    if (!(mpeg_fmt = av_find_input_format("mpeg"))) {
> > +        return AVERROR_DEMUXER_NOT_FOUND;
> > +    }
> > +
> > +    if (!(mpeg_ctx = avformat_alloc_context())) {
> > +        return AVERROR(ENOMEM);
> > +    }
> > +
> > +    if (!(mpeg_buf = av_malloc(DVDVIDEO_BLOCK_SIZE))) {
> > +        avformat_free_context(mpeg_ctx);
> > +
> > +        return AVERROR(ENOMEM);
> > +    }
> Why are you doing this instead of just declaring it to be a protocol
> that produces a mpeg-ps stream? That way it stops being your
> responsibility.
>
>
> > +
> > +    ffio_init_context(&c->mpeg_pb, mpeg_buf, DVDVIDEO_BLOCK_SIZE, 0,
> > +            s, dvdvideo_subdemux_read_data, NULL,
> dvdvideo_subdemux_seek_data);
> > +    c->mpeg_pb.pub.seekable = 0;
> > +
> > +    if ((ret = ff_copy_whiteblacklists(mpeg_ctx, s)) != 0) {
> > +        avformat_free_context(mpeg_ctx);
> > +
> > +        return ret;
> > +    }
> > +
> > +    /* TODO: fix the PTS issues (ask about this in ML) */
> > +    /* AVFMT_FLAG_GENPTS works in some scenarios, but in others, the
> reading process hangs */
> > +    mpeg_ctx->flags = AVFMT_FLAG_CUSTOM_IO | AVFMT_FLAG_GENPTS;
> MPEG-PS contains pts info, why not just use that? Not amazing PTS info,
> but usable PTS info.
>
> > +
> > +    mpeg_ctx->probesize = 0;
> > +    mpeg_ctx->max_analyze_duration = 0;
> > +
> > +    mpeg_ctx->interrupt_callback = s->interrupt_callback;
> > +    mpeg_ctx->pb = &c->mpeg_pb.pub;
> > +    mpeg_ctx->io_open = dvdvideo_subdemux_nested_io_open;
> > +
> > +    if ((ret = avformat_open_input(&mpeg_ctx, "", mpeg_fmt, NULL)) !=
> 0) {
> > +        avformat_free_context(mpeg_ctx);
> > +
> > +        return ret;
> > +    }
> > +
> > +    c->mpeg_fmt = mpeg_fmt;
> > +    c->mpeg_ctx = mpeg_ctx;
> > +    c->mpeg_buf = mpeg_buf;
> > +
> > +    return 0;
> > +}
> > +
> > +static void dvdvideo_subdemux_close(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    av_freep(&c->mpeg_pb.pub.buffer);
> > +    memset(&c->mpeg_pb, 0x00, sizeof(c->mpeg_pb));
> > +    av_freep(&c->mpeg_pb);Why are you zeroing it before you free it? If
> this is a security issue
> you can't use memset anyway as the compiler may throw it away. That's
> why explicit_bzero was created.
>
> > +
> > +    avformat_close_input(&c->mpeg_ctx);
> > +}
> > +
> > +static int dvdvideo_chapters_register(AVFormatContext *s)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +    int start_time = 0;
> > +    int cell = 0;
> > +
> > +    for (int i = 0; i < c->play_pgc->nr_of_programs; i++) {
> > +        int64_t ms = 0;
> > +        int next = c->play_pgc->program_map[i + 1];
> In the last iteration of the loop, will this possibly give you an
> out-of-bounds read?
>
> > +
> > +        if (i == c->play_pgc->nr_of_programs - 1) {
> > +            next = c->play_pgc->nr_of_cells + 1;
> > +        }
> > +
> > +        while (cell < next - 1) {
> > +            /* only consider first cell of multi-angle cells */
> > +            if (c->play_pgc->cell_playback[cell].block_mode <= 1) {
> > +                ms = ms + dvdvideo_time_to_millis(
> > +
> &c->play_pgc->cell_playback[cell].playback_time);
> > +            }
> > +            cell++;
> > +        }
> > +
> > +        if (!avpriv_new_chapter(s, i, DVDVIDEO_TIME_BASE_MILLIS_Q,
> > +                start_time, start_time + ms, NULL)) {
> > +            av_log(s, AV_LOG_ERROR, "Unable to allocate chapter");
> > +            return AVERROR(ENOMEM);
> > +        }
> > +
> > +        start_time += ms;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_read_header(AVFormatContext *s)
> > +{
> > +    int ret;
> > +
> > +    if ((ret = dvdvideo_volume_open(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_playback_open(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_subdemux_open(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_video_stream_register(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_audio_stream_register_all(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_subtitle_stream_register_all(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    if ((ret = dvdvideo_chapters_register(s)) != 0) {
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_read_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +    DVDVideoDemuxContext *c = s->priv_data;
> > +
> > +    while (!ff_check_interrupt(&s->interrupt_callback)) {
> > +        int subdemux_ret;
> > +        AVPacket *subdemux_pkt;
> > +
> > +        subdemux_pkt = av_packet_alloc();
> > +        if (!subdemux_pkt) {
> > +            return AVERROR(ENOMEM);
> > +        }
> > +
> > +        subdemux_ret = av_read_frame(c->mpeg_ctx, subdemux_pkt);
> > +        if (subdemux_ret >= 0) {
> > +            if (c->mpeg_ctx->nb_streams != s->nb_streams) {
> > +                av_log(s, AV_LOG_ERROR, "Unexpected stream during
> playback\n");
> > +
> > +                av_packet_unref(subdemux_pkt);
> > +
> > +                return AVERROR_INPUT_CHANGED;
> > +            }
> > +
> > +            av_packet_move_ref(pkt, subdemux_pkt);
> This seems like an unnecessary memcpy.
>
> > +
> > +            return 0;
> > +        }
> > +
> > +        return subdemux_ret;
> > +    }
> > +
> > +    return AVERROR_EOF;
> > +}
> > +
> > +static int dvdvideo_read_seek(AVFormatContext *s, int stream_index,
> > +        int64_t timestamp, int flags)
> > +{
> > +    av_log(s, AV_LOG_ERROR, "dvdvideo_read_seek(): not implemented\n");
> > +
> > +    return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +static int dvdvideo_close(AVFormatContext *s)
> > +{
> > +    dvdvideo_subdemux_close(s);
> > +    dvdvideo_playback_close(s);
> > +    dvdvideo_volume_close(s);
> > +
> > +    return 0;
> > +}
> > +
> > +static int dvdvideo_probe(const AVProbeData *p)
> > +{
> > +    av_log(NULL, AV_LOG_ERROR, "dvdvideo_probe(): not implemented\n");
> > +
> > +    return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +#define OFFSET(x) offsetof(DVDVideoDemuxContext, x)
> > +static const AVOption dvdvideo_options[] = {
> > +    {"title",   "Title Number",                     OFFSET(opt_title),
>     AV_OPT_TYPE_INT,    { .i64=1 },     1,
> DVDVIDEO_TITLE_NUMBER_MAX,          AV_OPT_FLAG_DECODING_PARAM },
> > +    {"ptt",     "Entry PTT Number",                 OFFSET(opt_ptt),
>     AV_OPT_TYPE_INT,    { .i64=1 },     1,
> DVDVIDEO_PTT_NUMBER_MAX,            AV_OPT_FLAG_DECODING_PARAM },
> > +    {"pgc",     "Entry PGC Number (0=auto)",        OFFSET(opt_pgc),
>     AV_OPT_TYPE_INT,    { .i64=0 },     0,
> DVDVIDEO_PGC_NUMBER_MAX,            AV_OPT_FLAG_DECODING_PARAM },
> > +    {"pg",      "Entry PG Number (0=auto)",         OFFSET(opt_pg),
>      AV_OPT_TYPE_INT,    { .i64=0 },     0,
> DVDVIDEO_PG_NUMBER_MAX,             AV_OPT_FLAG_DECODING_PARAM },
> > +    {"angle",   "Video Angle Number",               OFFSET(opt_angle),
>     AV_OPT_TYPE_INT,    { .i64=1 },     1,
> DVDVIDEO_ANGLE_NUMBER_MAX,          AV_OPT_FLAG_DECODING_PARAM },
> > +    {"region",  "Playback Region Number (0=free)",
> OFFSET(opt_region),     AV_OPT_TYPE_INT,    { .i64=0 },     0,
> DVDVIDEO_REGION_NUMBER_MAX,         AV_OPT_FLAG_DECODING_PARAM },
> > +    {NULL}
> > +};
> > +
> > +static const AVClass dvdvideo_class = {
> > +    .class_name = "DVD-Video demuxer",
> > +    .item_name  = av_default_item_name,
> > +    .option     = dvdvideo_options,
> > +    .version    = LIBAVUTIL_VERSION_INT
> > +};
> > +
> > +const AVInputFormat ff_dvdvideo_demuxer = {
> > +    .name           = "dvdvideo",
> > +    .long_name      = NULL_IF_CONFIG_SMALL("DVD-Video"),
> > +    .priv_class     = &dvdvideo_class,
> > +    .priv_data_size = sizeof(DVDVideoDemuxContext),
> > +    .flags          = AVFMT_NOFILE | AVFMT_NO_BYTE_SEEK |
> AVFMT_NOGENSEARCH | AVFMT_NOBINSEARCH | AVFMT_SHOW_IDS | AVFMT_TS_DISCONT,
> > +    .flags_internal = FF_FMT_INIT_CLEANUP,
> > +    .read_probe     = dvdvideo_probe,
> > +    .read_close     = dvdvideo_close,
> > +    .read_header    = dvdvideo_read_header,
> > +    .read_packet    = dvdvideo_read_packet,
> > +    .read_seek      = dvdvideo_read_seek
> > +};
>
> There's a number of other issues and I didn't write each one of each
> type, but my biggest question is why this is not a protocol that
> produces an mpeg-ps stream.
>
> - Leo Izen (Traneptora)
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>


More information about the ffmpeg-devel mailing list