Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Jan Ekström" <jeebjp@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] mlp_parser: fetch a new timestamp when major sync is found
Date: Tue, 8 Mar 2022 00:02:55 +0200
Message-ID: <CAEu79Sb0QYt+afnqkbK3FTuS44mqAtDjho=AYf4jD0F103wYrw@mail.gmail.com> (raw)
In-Reply-To: <20220307171019.GK2829255@pb2>

On Mon, Mar 7, 2022 at 7:10 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Mon, Mar 07, 2022 at 12:32:59AM +0200, Jan Ekström wrote:
> > From: Hendrik Leppkes <h.leppkes@gmail.com>
> >
> > Decoding can only start on a major sync
>
> > and the parser discards
> > any data received until that point.
>
> Thats a bug and its inconsistant
> For example video codec parsers do not drop frames until a keyframe
> And droping that in a video codec would if someone suggested it
> lead to people opposing as that would drop actual data
> that can partly be decoded. Its better to drop it in the decoder
> if its unwanted.
> So why is this considered to be "ok" in audio codecs?
> Because we dont visually see that data is lost ?
>

For the record, I am not advocating for this behavior in this parser,
nor is this patch adding the dropping. The dropping is and has already
been there.

Rather this is just the result of the following steps:

1. I improve certain parser calling code in an API client (mpv) to
actually utilize the returned timestamps due to certain formats such
as E-AC-3 having buffering/delay.
2. Some of the samples I have at hand were badly cut matroska files
from early 2010s where the muxer expected all packets to be random
access points.
3. I notice that while the E-AC-3 samples' timestamps are now correct,
the TrueHD samples' timestamps are now such that you get a jump
between the first returned data, and the rest of the returned data.
4. After staring at the matroska packets' sizes and timestamps on the
container level, I figure out that the parser is just dropping packets
until the first random access point packet is found, and then failing
to update the returned timestamp.
5. Thus I open https://trac.ffmpeg.org/ticket/9428 .
6. nev is nice enough to figure out how to make the parser actually
update the returned timestamp.
7. After the copyinkf stuff gets applied to master, I write a test for
it and reword the commit message, send the fix to the mailing list.

I do not have experience with the parser, but at least this fixes an
existing bug that I noticed when properly utilizing the parsing API :)
.

> So let me show it vissually :)
> i deleted frame 1, we have 3 decoder outputs
> 1. complete data
> 2. no frame 1 but instead frame 17 duplicated in frame 1s place
> 3. no frame 1
>
> The decoder produces a few errors in case 2 but there seems more
> data coming out which seems resembling the data that is droped
> its not 1:1 identical but it looks like there is recoverable data
> see attached screenshoot from audacity
> Maybe its a coincidence
>
> but i dont think we should just build on top of this droping
> logic without some argumentation why this droped data can really
> not be used ever in any way (it may become harder to fix it the
> more is build on top)
>
> thx
>

I'm OK with the parser being changed in stead, but unfortunately I do
not have the brain power/time to start looking into it myself.

For now I was happy enough to get a fix for the returned timestamps
posted on the mailing list, and have written a test for it.

Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

      parent reply	other threads:[~2022-03-07 22:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-06 22:32 Jan Ekström
2022-03-07 17:10 ` Michael Niedermayer
2022-03-07 21:55   ` Hendrik Leppkes
2022-03-07 22:02   ` Jan Ekström [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEu79Sb0QYt+afnqkbK3FTuS44mqAtDjho=AYf4jD0F103wYrw@mail.gmail.com' \
    --to=jeebjp@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git