[FFmpeg-devel] [RFC/PATCH] Pass PRIVATE_STREAM_2 MPEG-PS packets to caller
Michael Niedermayer
michaelni at gmx.at
Fri Mar 8 02:00:58 CET 2013
On Fri, Mar 08, 2013 at 01:18:21AM +0100, Richard wrote:
> On 03/03/13 16:09, Richard wrote:
> >On 03/03/13 10:25, Richard wrote:
> >>On 03/03/13 01:55, Michael Niedermayer wrote:
> >>>On Sun, Mar 03, 2013 at 12:51:46AM +0100, Richard wrote:
> >>>>On 02/03/13 13:18, Michael Niedermayer wrote:
> >>>[...]
> >>>>+static int dvd_nav_parse(AVCodecParserContext *s,
> >>>>+ AVCodecContext *avctx,
> >>>>+ const uint8_t **poutbuf, int *poutbuf_size,
> >>>>+ const uint8_t *buf, int buf_size)
> >>>>+{
> >>>>+ DVDNavParseContext *pc1 = s->priv_data;
> >>>>+ ParseContext *pc = &pc1->pc;
> >>>>+ int lastPacket = 0;
> >>>>+ int valid = 0;
> >>>>+
> >>>>+ s->pict_type = AV_PICTURE_TYPE_NONE;
> >>>>+
> >>>
> >>>>+ avctx->time_base.num = 1;
> >>>>+ avctx->time_base.den = 90000;
> >>>
> >>>why is this needed ?
> >>
> >>Because otherwise the AVPacket returned to the calling application ends
> >>up with a duration of 0.
> >>
> >><snip>
> >>
> >>>>+ }
> >>>>+ }
> >>>>+
> >>>>+ if (!valid || lastPacket) {
> >>>>+ pc1->copied = 0;
> >>>>+ pc1->lba = 0xFFFFFFFF;
> >>>>+ }
> >>>
> >>>the {} placement in this file is rather inconsistent
> >>
> >>Yes, you're right. Sorry, I was trying to keep to K&R style, which
> >>seems to be the main style used, but which isn't my normal style. I'll
> >>tidy that up.
> >>
> >><snip>
> >>
> >>>>diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> >>>>index 4eaffd8..98ddc88 100644
> >>>>--- a/libavformat/mpeg.c
> >>>>+++ b/libavformat/mpeg.c
> >>>>@@ -247,9 +247,12 @@ static int
> >>>>mpegps_read_pes_header(AVFormatContext *s,
> >>>> goto redo;
> >>>> }
> >>>> if (startcode == PRIVATE_STREAM_2) {
> >>>>- len = avio_rb16(s->pb);
> >>>>+ int origlen = len = avio_rb16(s->pb);
> >>>>+ uint8_t firstbyte = avio_r8(s->pb);
> >>>>+ avio_skip(s->pb, -1);
> >>>> if (!m->sofdec) {
> >>>>- while (len-- >= 6) {
> >>>>+ while (len >= 6) {
> >>>>+ len--;
> >>>> if (avio_r8(s->pb) == 'S') {
> >>>> uint8_t buf[5];
> >>>> avio_read(s->pb, buf, sizeof(buf));
> >>>>@@ -260,8 +263,15 @@ static int
> >>>>mpegps_read_pes_header(AVFormatContext *s,
> >>>> }
> >>>> m->sofdec -= !m->sofdec;
> >>>> }
> >>>>- avio_skip(s->pb, len);
> >>>>- goto redo;
> >>>>+ if (m->sofdec <= 0 &&
> >>>>+ ((origlen == 980 && firstbyte == 0) ||
> >>>>+ (origlen == 1018 && firstbyte == 1))) {
> >>>>+ /* DVD NAV packet, move back to the start of the stream
> >>>>(plus 'length' field) */
> >>>
> >>>>+ avio_skip(s->pb, -((origlen-len) + 2));
> >>>
> >>>this seeks backward which is not guranteed to work on non seekable
> >>>protocols. On a dvd it would work but might if the OS cannot satisfy
> >>>it from some cache do an actual seek which is slow.
> >>>the same issue exists a fiw lines above
> >>
> >>Yes, I wasn't completely happy with that myself. The problem is that
> >>the contents need to be inspected to determine the codec for the packet
> >>(in the case of a DVD) or the codec to use for audio (in the case of
> >>Sofdec). But inspecting the contents here prevents them being passed on
> >>without a negative seek. How should/can that be solved here?
> >
> >Ok, attached is a patch that addresses the formatting/code style issues.
>
> And another that hopefully successfully addresses the negative seek
> issue. Once a DVD stream has been determined, no more packet
> inspection is performed, so any seeks will only happen once, when
> the first PRIVATE_STREAM_2 packet is detected.
>
> Richard.
>
> libavcodec/Makefile | 1
> libavcodec/allcodecs.c | 1
> libavcodec/avcodec.h | 2
> libavcodec/codec_desc.c | 6 ++
> libavcodec/dvd_nav_parser.c | 118 ++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/version.h | 2
> libavformat/mpeg.c | 40 ++++++++++++--
> 7 files changed, 163 insertions(+), 7 deletions(-)
> ebaf3ac5186dbc6c20539a9d4100b03b7976b2c5 0001-Add-passing-DVD-navigation-packets-startcode-0x1bf-t.patch
> From cea0c4411f11979bee6289ea5abbcd6d2e7e692c Mon Sep 17 00:00:00 2001
> From: Richard <peper03 at yahoo.com>
> Date: Fri, 8 Mar 2013 01:07:59 +0100
> Subject: [PATCH] Add passing DVD navigation packets (startcode 0x1bf) to
> caller to allow better playback handling of DVDs. The two
> types of packets (PCI and DSI) are passed untouched but
> combined by the new codec ID AV_CODEC_ID_DVD_NAV. The
> first 980 bytes in the packet contain the PCI data. The
> next 1018 are the DSI data.
>
> ---
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 2 +
> libavcodec/codec_desc.c | 6 +++
> libavcodec/dvd_nav_parser.c | 118 +++++++++++++++++++++++++++++++++++++++++++
> libavcodec/version.h | 2 +-
> libavformat/mpeg.c | 40 ++++++++++++---
> 7 files changed, 163 insertions(+), 7 deletions(-)
> create mode 100644 libavcodec/dvd_nav_parser.c
[...]
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 67fdc25..1e08f09 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
> #include "libavutil/avutil.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 54
> -#define LIBAVCODEC_VERSION_MINOR 92
> +#define LIBAVCODEC_VERSION_MINOR 93
> #define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> index 4eaffd8..c890b0c 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -108,6 +108,7 @@ typedef struct MpegDemuxContext {
> int32_t header_state;
> unsigned char psm_es_type[256];
> int sofdec;
> + int dvd;
> #if CONFIG_VOBSUB_DEMUXER
> AVFormatContext *sub_ctx;
> FFDemuxSubtitlesQueue q;
> @@ -247,9 +248,13 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> goto redo;
> }
> if (startcode == PRIVATE_STREAM_2) {
> - len = avio_rb16(s->pb);
> - if (!m->sofdec) {
> - while (len-- >= 6) {
> + if (!m->sofdec && !m->dvd) {
> + int origlen = len = avio_rb16(s->pb);
> + uint8_t firstbyte = avio_r8(s->pb);
> + avio_skip(s->pb, -1);
> +
> + while (len >= 6) {
> + len--;
> if (avio_r8(s->pb) == 'S') {
this reads a byte then seeks back and reads it again
this sure can be done without the seek
> uint8_t buf[5];
> avio_read(s->pb, buf, sizeof(buf));
> @@ -259,9 +264,24 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> }
> }
> m->sofdec -= !m->sofdec;
> +
> + if (m->sofdec <= 0 &&
> + ((origlen == 980 && firstbyte == 0) ||
> + (origlen == 1018 && firstbyte == 1))) {
> + /* DVD NAV packet, move back to the start of the stream (plus 'length' field) */
> + avio_skip(s->pb, -((origlen-len) + 2));
this needs a failure check and the failure must be handled
> + m->dvd = 1;
> + } else {
> + avio_skip(s->pb, len);
> + goto redo;
> + }
> + }
> +
> + if (!m->dvd) {
> + len = avio_rb16(s->pb);
> + avio_skip(s->pb, len);
> + goto redo;
> }
> - avio_skip(s->pb, len);
> - goto redo;
> }
> if (startcode == PROGRAM_STREAM_MAP) {
> mpegps_psm_parse(m, s->pb);
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130308/a29b822e/attachment.asc>
More information about the ffmpeg-devel
mailing list