[FFmpeg-devel] [RFC/PATCH] Pass PRIVATE_STREAM_2 MPEG-PS packets to caller
Michael Niedermayer
michaelni at gmx.at
Sat Mar 16 22:08:11 CET 2013
On Sat, Mar 16, 2013 at 09:29:00PM +0100, Richard wrote:
> On 16/03/13 01:16, Michael Niedermayer wrote:
> >On Sat, Mar 16, 2013 at 12:08:16AM +0100, Richard wrote:
> >>On 15/03/13 21:50, Michael Niedermayer wrote:
> >>>On Fri, Mar 15, 2013 at 11:28:03AM +0100, Richard wrote:
> >>>>On 15/03/13 00:10, Michael Niedermayer wrote:
> >>>>>On Fri, Mar 08, 2013 at 09:41:50PM +0100, Richard wrote:
> >>>>>>+
> >>>>>>+ while (len >= 6) {
> >>>>>>+ len--;
> >>>>>> if (avio_r8(s->pb) == 'S') {
> >>>>>> uint8_t buf[5];
> >>>>>> avio_read(s->pb, buf, sizeof(buf));
> >>>>>>@@ -259,9 +269,32 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> >>>>>> }
> >>>>>> }
> >>>>>> m->sofdec -= !m->sofdec;
> >>>>>
> >>>>>you change sofdec detection behavior here
> >>>>>before it run once, after your patch it runs on every packet
> >>>>
> >>>>No it doesn't. There's still the check for '!m->sofdec', it's just
> >>>>been extended to also check for '!m->dvd'. Once the first packet
> >>>>has been checked, m->sofdec is either 1 or -1, so that code will
> >>>>only ever be executed once.
> >>>
> >>>ok but then why is the check for dvd == 0 there ?
> >>>its 0 on the first run and irrelevant after
> >>
> >>Because if we're not dealing with a DVD, we should skip that packet
> >>completely, just like before my patch. It's not irrelevant after.
> >>
> >>On the first run, sofdec is zero and dvd is zero, so we enter the
> >>'checking code'. That will set sofdec to either -1 (not sofdec) or
> >>1 (is sofdec), and if sofdec is not detected but a DVD packet is,
> >>dvd will be set to 1 so the possibilities after running the checking
> >>code on the first packet are:
> >>
> >>sofdec = -1
> >>dvd = 0
> >> = Unidentified packet - skip
> >>
> >>sofdec = 1
> >>dvd = 0
> >> = Sofdec packet - skip
> >>
> >>sofdec = -1
> >>dvd = 1
> >> = DVD packet, don't skip
> >
> >the code in your patch is this:
> >if (!m->sofdec && !m->dvd) {
> >
> >it results in
> >0, 0 and 0 for these 3 cases and 1 for the initial state
> >
> >if (!m->sofdec)
> >
> >does the same
>
> No it doesn't. m->sofdec is changed in the check code. It will be
> zero on the way in if it's the first packet but it definitely won't
> be zero on the way out. That second 'if' is checking the value
> *after* the check code.
try:
+av_assert0((!m->sofdec && !m->dvd) == !m->sofdec);
if (!m->sofdec && !m->dvd) {
[...]
> +#define PCI_SIZE 980
> +#define DSI_SIZE 1018
> +
> +/* parser definition */
> +typedef struct DVDNavParseContext {
> + ParseContext pc;
> + uint32_t lba;
> + uint8_t buffer[PCI_SIZE+DSI_SIZE];
> + int copied;
> +} DVDNavParseContext;
> +
> +static av_cold int dvd_nav_parse_init(AVCodecParserContext *s)
> +{
> + DVDNavParseContext *pc = s->priv_data;
> +
> + pc->lba = 0xFFFFFFFF;
> + pc->copied = 0;
> + return 0;
> +}
> +
> +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;
unused
> + int lastPacket = 0;
> + int valid = 0;
> +
> + s->pict_type = AV_PICTURE_TYPE_NONE;
> +
> + avctx->time_base.num = 1;
> + avctx->time_base.den = 90000;
> +
> + if (buf && buf_size) {
> + switch(buf[0]) {
> + case 0x00:
> + if (buf_size == PCI_SIZE) {
> + /* PCI */
> + uint32_t lba = AV_RB32(&buf[0x01]);
> + uint32_t startpts = AV_RB32(&buf[0x0D]);
> + uint32_t endpts = AV_RB32(&buf[0x11]);
> +
> + if (endpts > startpts) {
> + pc1->lba = lba;
> + s->pts = (int64_t)startpts;
> + s->duration = endpts - startpts;
> +
> + memcpy(pc1->buffer, buf, PCI_SIZE);
> + pc1->copied = PCI_SIZE;
> + valid = 1;
> + }
> + }
> + break;
> +
> + case 0x01:
> + if ((buf_size == DSI_SIZE) && (pc1->copied == PCI_SIZE)) {
> + /* DSI */
> + uint32_t lba = AV_RB32(&buf[0x05]);
> +
> + if (lba == pc1->lba) {
> + memcpy(pc1->buffer + pc1->copied, buf, DSI_SIZE);
> + lastPacket = 1;
> + valid = 1;
> + }
> + }
> + break;
> + }
> + }
> +
> + if (!valid || lastPacket) {
> + pc1->copied = 0;
> + pc1->lba = 0xFFFFFFFF;
> + }
> +
> + if (lastPacket) {
> + *poutbuf = pc1->buffer;
> + *poutbuf_size = sizeof(pc1->buffer);
> + } else {
> + *poutbuf = NULL;
> + *poutbuf_size = 0;
> + }
> +
> + return buf_size;
> +}
> +
> +AVCodecParser ff_dvd_nav_parser = {
> + .codec_ids = { AV_CODEC_ID_DVD_NAV },
> + .priv_data_size = sizeof(DVDNavParseContext),
> + .parser_init = dvd_nav_parse_init,
> + .parser_parse = dvd_nav_parse,
> +};
> 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..5b2e8d2 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -30,6 +30,7 @@
>
> #undef NDEBUG
> #include <assert.h>
> +#include "libavutil/avassert.h"
>
> /*********************************************/
> /* demux code */
> @@ -108,6 +109,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,21 +249,67 @@ 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 (avio_r8(s->pb) == 'S') {
> - uint8_t buf[5];
> - avio_read(s->pb, buf, sizeof(buf));
> - m->sofdec = !memcmp(buf, "ofdec", 5);
> - len -= sizeof(buf);
> - break;
> + if (!m->sofdec && !m->dvd) {
> + /* Need to detect whether this from a DVD or a 'Sofdec' stream */
> + int len = avio_rb16(s->pb);
> + uint8_t *ps2buf = (uint8_t*)av_malloc(len);
unneeded cast
> + uint8_t *p;
> + av_assert0(ps2buf);
> +
> + if( avio_read(s->pb, ps2buf, len) < 0 )
> + av_assert0(0);
you cannot abort the lib and user application through assert(0) on an
IO or allocation error
> +
> + p = memchr(ps2buf, 'S', len - 6);
missing check that len >= 6
also it should be len - 5
> + if(p)
> + m->sofdec = !memcmp(p+1, "ofdec", 5);
> +
> + m->sofdec -= !m->sofdec;
> +
> + if (m->sofdec < 0) {
> + if (len == 980 && ps2buf[0] == 0) {
> + /* PCI structure? */
> + uint32_t startpts = AV_RB32(ps2buf + 0x0d);
> + uint32_t endpts = AV_RB32(ps2buf + 0x11);
> + uint8_t hours = ((ps2buf[0x19] >> 4) * 10) + (ps2buf[0x19] & 0x0f);
0xA and 0x10 would both result in 10 hours ane be considered valid,
i suspect that is not intended
> + uint8_t mins = ((ps2buf[0x1a] >> 4) * 10) + (ps2buf[0x1a] & 0x0f);
> + uint8_t secs = ((ps2buf[0x1b] >> 4) * 10) + (ps2buf[0x1b] & 0x0f);
> +
> + m->dvd = (hours <= 23 &&
> + mins <= 59 &&
> + secs <= 59 &&
> + endpts >= startpts);
> + } else if (len == 1018 && ps2buf[0] == 1) {
> + /* DSI structure? */
> + uint8_t hours = ((ps2buf[0x1d] >> 4) * 10) + (ps2buf[0x1d] & 0x0f);
> + uint8_t mins = ((ps2buf[0x1e] >> 4) * 10) + (ps2buf[0x1e] & 0x0f);
> + uint8_t secs = ((ps2buf[0x1f] >> 4) * 10) + (ps2buf[0x1f] & 0x0f);
> +
> + m->dvd = (hours <= 23 &&
> + mins <= 59 &&
> + secs <= 59);
> }
> +
> + av_free(ps2buf);
> +
> + /* If this isn't a DVD packet, just ignore it.
> + * If we did, move back to the start of the
> + * packet (plus 'length' field) */
> + if (!m->dvd || avio_skip(s->pb, -(len + 2)) < 0) {
> + /* Skip back failed.
> + * This packet will be lost but that can't be helped
> + * if we can't skip back
> + */
> + goto redo;
> + }
> + } else {
> + /* Sofdec detected */
> + goto redo;
> }
memleak of ps2buf
> - m->sofdec -= !m->sofdec;
> + } else if (!m->dvd) {
> + int 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);
> @@ -271,7 +319,9 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> /* find matching stream */
> if (!((startcode >= 0x1c0 && startcode <= 0x1df) ||
> (startcode >= 0x1e0 && startcode <= 0x1ef) ||
> - (startcode == 0x1bd) || (startcode == 0x1fd)))
> + (startcode == 0x1bd) ||
> + (startcode == PRIVATE_STREAM_2) ||
> + (startcode == 0x1fd)))
> goto redo;
> if (ppos) {
> *ppos = avio_tell(s->pb) - 4;
> @@ -279,6 +329,8 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> len = avio_rb16(s->pb);
> pts =
> dts = AV_NOPTS_VALUE;
> + if (startcode != PRIVATE_STREAM_2)
> + {
> /* stuffing */
> for(;;) {
> if (len < 1)
> @@ -352,6 +404,7 @@ static int mpegps_read_pes_header(AVFormatContext *s,
> }
> else if( c!= 0xf )
> goto redo;
> + }
>
> if (startcode == PRIVATE_STREAM_1) {
> startcode = avio_r8(s->pb);
> @@ -448,6 +501,9 @@ static int mpegps_read_packet(AVFormatContext *s,
> else
> request_probe= 1;
> type = AVMEDIA_TYPE_VIDEO;
> + } else if (startcode == PRIVATE_STREAM_2) {
> + type = AVMEDIA_TYPE_DATA;
> + codec_id = AV_CODEC_ID_DVD_NAV;
> } else if (startcode >= 0x1c0 && startcode <= 0x1df) {
> type = AVMEDIA_TYPE_AUDIO;
> codec_id = m->sofdec > 0 ? AV_CODEC_ID_ADPCM_ADX : AV_CODEC_ID_MP2;
> --
> 1.7.9.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20130316/a0a52576/attachment.asc>
More information about the ffmpeg-devel
mailing list