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

Marth64 marth64 at proxyid.net
Sun Dec 10 05:16:55 EET 2023


Hi Leo,

Thank you for the feedback and review. I will work through the code style
issues. Please do let me know what else you find - I will fix.

Here is some responses on the conceptual issues,

> my biggest question is why this is not a protocol that produces an
mpeg-ps stream.
Protocol handler in its current state does not allow revealing of language
codes or chapters AFAIK.
Also, there are streams in DVD that may pop up later in the lifecycle such
as forced subtitles used in the middle of a program.

> Why do you need both of these? libdvdnav depends on libdvdread.
This is because I need access to libdvdread headers in order to probe the
stream information in advance of the playback. Specifically, I need access
to what all could be in the stream vs. what the VM in libdvdnav thinks it
is currently playing back.

> Unnecessary #define.
I will get rid of it, np

> 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.
> This is wrong, it's 30000/1001. Which you knew, because you #defined it
above. Why not use those?
Sure, I will simplify this. I will fix the chapter timing function anyways
as I am now noticing it is very slightly off.

> Can this fail? (x2)
> Our convention is to return negative values on error in FFmpeg. Not
nonzero values.
Both have a return type void.  However your question caused me to also
check dvdnav_close() which can fail with a status. So I will fix that,
along with the potential non-zero response and being explicit to prevent it.

> This seems like an unnecessary memcpy.
I can do without it. I assumed it was a good idea to create some distance
between my demuxer's packet and the MPEG PS packet but I guess there is no
value to your point.

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