| Message ID | 20190211165934.13075-1-guillaume.desmottes@collabora.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Feb 11, 2019 at 05:59:34PM +0100, Guillaume Desmottes wrote: > CDG doesn't ensure a constant framerate as we can have holes in the CDG > stream. So there is no guarantee of the duration of a single frame, it > will be displayed until a new packet with CDG instruction arrives in the > stream. > > Signed-off-by: Guillaume Desmottes <guillaume.desmottes@collabora.com> > --- > libavformat/cdg.c | 1 + > 1 file changed, 1 insertion(+) breaks fate-cdgraphics [...]
On 11/02/2019 23:41, Michael Niedermayer wrote:
> breaks fate-cdgraphics
Hi Michael,
Thanks for testing my patch. I wasn't aware of this specific test, sorry
about that.
What would be the proper way to address this? Should the fate reference
be updated?
The test file is still properly played by ffplay so playback isn't broken.
This is my first ffmpeg patch so any pointer to help me getting this
patch integrated would be really helpful.
Thanks a lot,
G.
2019-02-13 16:19 GMT+01:00, Guillaume Desmottes <guillaume.desmottes@collabora.com>: > Thanks for testing my patch. I wasn't aware of this specific test, sorry > about that. > > What would be the proper way to address this? Should the fate reference > be updated? Yes, unless the change is wrong. Carl Eugen
On 13/02/2019 16:28, Carl Eugen Hoyos wrote: >> Thanks for testing my patch. I wasn't aware of this specific test, sorry >> about that. >> >> What would be the proper way to address this? Should the fate reference >> be updated? > Yes, unless the change is wrong. Ok thanks. So, who should I ask to review this patch and update the reference then?
On 2/11/2019 1:59 PM, Guillaume Desmottes wrote: > CDG doesn't ensure a constant framerate as we can have holes in the CDG > stream. So there is no guarantee of the duration of a single frame, it > will be displayed until a new packet with CDG instruction arrives in the > stream. > > Signed-off-by: Guillaume Desmottes <guillaume.desmottes@collabora.com> > --- > libavformat/cdg.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavformat/cdg.c b/libavformat/cdg.c > index 05cac6e528..078e985223 100644 > --- a/libavformat/cdg.c > +++ b/libavformat/cdg.c > @@ -74,6 +74,7 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) > pkt->stream_index = 0; > pkt->dts= > pkt->pts= pkt->pos / CDG_PACKET_SIZE; > + pkt->duration = AV_NOPTS_VALUE; The doxy says "0 if unknown", so AV_NOPTS_VALUE is not correct. > > if(ret>5 && (pkt->data[0]&0x3F) == 9 && (pkt->data[1]&0x3F)==1 && !(pkt->data[2+2+1] & 0x0F)){ > pkt->flags = AV_PKT_FLAG_KEY; >
Hi James, On 19/02/2019 17:14, James Almer wrote: > On 2/11/2019 1:59 PM, Guillaume Desmottes wrote: >> CDG doesn't ensure a constant framerate as we can have holes in the CDG >> stream. So there is no guarantee of the duration of a single frame, it >> will be displayed until a new packet with CDG instruction arrives in the >> stream. >> >> Signed-off-by: Guillaume Desmottes <guillaume.desmottes@collabora.com> >> --- >> libavformat/cdg.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/libavformat/cdg.c b/libavformat/cdg.c >> index 05cac6e528..078e985223 100644 >> --- a/libavformat/cdg.c >> +++ b/libavformat/cdg.c >> @@ -74,6 +74,7 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) >> pkt->stream_index = 0; >> pkt->dts= >> pkt->pts= pkt->pos / CDG_PACKET_SIZE; >> + pkt->duration = AV_NOPTS_VALUE; > The doxy says "0 if unknown", so AV_NOPTS_VALUE is not correct. Just tried that, but by setting it to 0, compute_pkt_fields() set it back to an actual value which is exactly what I'm trying to avoid. See https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/utils.c#L1303
diff --git a/libavformat/cdg.c b/libavformat/cdg.c index 05cac6e528..078e985223 100644 --- a/libavformat/cdg.c +++ b/libavformat/cdg.c @@ -74,6 +74,7 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) pkt->stream_index = 0; pkt->dts= pkt->pts= pkt->pos / CDG_PACKET_SIZE; + pkt->duration = AV_NOPTS_VALUE; if(ret>5 && (pkt->data[0]&0x3F) == 9 && (pkt->data[1]&0x3F)==1 && !(pkt->data[2+2+1] & 0x0F)){ pkt->flags = AV_PKT_FLAG_KEY;
CDG doesn't ensure a constant framerate as we can have holes in the CDG stream. So there is no guarantee of the duration of a single frame, it will be displayed until a new packet with CDG instruction arrives in the stream. Signed-off-by: Guillaume Desmottes <guillaume.desmottes@collabora.com> --- libavformat/cdg.c | 1 + 1 file changed, 1 insertion(+)