Re: [FFmpeg-devel] [PATCH 1/1] Sega FILM: set dts and duration when demuxing

[Michael Niedermayer]

On Mon, Apr 09, 2018 at 04:11:43PM -0700, Kyle Swanson wrote:
> Hi,
> 
> On Sun, Apr 8, 2018 at 6:27 PM,  wrote:
> 
> > From: Misty De Meo 
> >
> > ---
> >  libavformat/segafilm.c | 19 +++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/libavformat/segafilm.c b/libavformat/segafilm.c
> > index 4c0cca0140..e72c26b144 100644
> > --- a/libavformat/segafilm.c
> > +++ b/libavformat/segafilm.c
> > @@ -270,6 +270,8 @@ static int film_read_packet(AVFormatContext *s,
> >  FilmDemuxContext *film = s->priv_data;
> >  AVIOContext *pb = s->pb;
> >  film_sample *sample;
> > +film_sample *next_sample = NULL;
> > +int next_sample_id;
> >  int ret = 0;
> >
> >  if (film->current_sample >= film->sample_count)
> > @@ -277,6 +279,20 @@ static int film_read_packet(AVFormatContext *s,
> >
> >  sample = >sample_table[film->current_sample];
> >
> > +/* Find the next sample from the same stream, assuming there is one;
> > + * this is used to calculate the duration below */
> > +next_sample_id = film->current_sample + 1;
> > +while (next_sample == NULL) {
> > +if (next_sample_id >= film->sample_count)
> > +break;
> > +
> > +next_sample = >sample_table[next_sample_id];
> > +if (next_sample->stream != sample->stream) {
> > +next_sample = NULL;
> > +next_sample_id++;
> > +}
> > +}
> > +
> >  /* position the stream (will probably be there anyway) */
> >  avio_seek(pb, sample->sample_offset, SEEK_SET);
> >
> > @@ -285,8 +301,11 @@ static int film_read_packet(AVFormatContext *s,
> >  ret = AVERROR(EIO);
> >
> >  pkt->stream_index = sample->stream;
> > +pkt->dts = sample->pts;
> >  pkt->pts = sample->pts;
> >  pkt->flags |= sample->keyframe ? AV_PKT_FLAG_KEY : 0;
> > +if (next_sample != NULL)
> > +pkt->duration = next_sample->pts - sample->pts;
> >
> >  film->current_sample++;
> >
> > --
> > 2.17.0
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> lgtm

will apply

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] Sega FILM: set dts and duration when demuxing

[Kyle Swanson]

Hi,

On Sun, Apr 8, 2018 at 6:27 PM,  wrote:

> From: Misty De Meo 
>
> ---
>  libavformat/segafilm.c | 19 +++
>  1 file changed, 19 insertions(+)
>
> diff --git a/libavformat/segafilm.c b/libavformat/segafilm.c
> index 4c0cca0140..e72c26b144 100644
> --- a/libavformat/segafilm.c
> +++ b/libavformat/segafilm.c
> @@ -270,6 +270,8 @@ static int film_read_packet(AVFormatContext *s,
>  FilmDemuxContext *film = s->priv_data;
>  AVIOContext *pb = s->pb;
>  film_sample *sample;
> +film_sample *next_sample = NULL;
> +int next_sample_id;
>  int ret = 0;
>
>  if (film->current_sample >= film->sample_count)
> @@ -277,6 +279,20 @@ static int film_read_packet(AVFormatContext *s,
>
>  sample = >sample_table[film->current_sample];
>
> +/* Find the next sample from the same stream, assuming there is one;
> + * this is used to calculate the duration below */
> +next_sample_id = film->current_sample + 1;
> +while (next_sample == NULL) {
> +if (next_sample_id >= film->sample_count)
> +break;
> +
> +next_sample = >sample_table[next_sample_id];
> +if (next_sample->stream != sample->stream) {
> +next_sample = NULL;
> +next_sample_id++;
> +}
> +}
> +
>  /* position the stream (will probably be there anyway) */
>  avio_seek(pb, sample->sample_offset, SEEK_SET);
>
> @@ -285,8 +301,11 @@ static int film_read_packet(AVFormatContext *s,
>  ret = AVERROR(EIO);
>
>  pkt->stream_index = sample->stream;
> +pkt->dts = sample->pts;
>  pkt->pts = sample->pts;
>  pkt->flags |= sample->keyframe ? AV_PKT_FLAG_KEY : 0;
> +if (next_sample != NULL)
> +pkt->duration = next_sample->pts - sample->pts;
>
>  film->current_sample++;
>
> --
> 2.17.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] Sega FILM: set dts and duration when demuxing

[Rostislav Pehlivanov]

On 22 March 2018 at 02:44,  wrote:

> From: Misty De Meo 
>
> ---
>  Changelog  |  1 +
>  libavformat/segafilm.c | 21 +
>  2 files changed, 22 insertions(+)
>
> diff --git a/Changelog b/Changelog
> index 7969b414c4..6b3b2bf7fe 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -47,6 +47,7 @@ version :
>  - native SBC encoder and decoder
>  - drmeter audio filter
>  - hapqa_extract bitstream filter
> +- segafilm: set timestamps when demuxing
>

Changes like these shouldn't be in the changelog.


>
>
>  version 3.4:
> diff --git a/libavformat/segafilm.c b/libavformat/segafilm.c
> index 11768823fc..2237e25dae 100644
> --- a/libavformat/segafilm.c
> +++ b/libavformat/segafilm.c
> @@ -270,6 +270,8 @@ static int film_read_packet(AVFormatContext *s,
>  FilmDemuxContext *film = s->priv_data;
>  AVIOContext *pb = s->pb;
>  film_sample *sample;
> +film_sample *next_sample = NULL;
> +int next_sample_id;
>  int ret = 0;
>
>  if (film->current_sample >= film->sample_count)
> @@ -277,6 +279,21 @@ static int film_read_packet(AVFormatContext *s,
>
>  sample = >sample_table[film->current_sample];
>
> +/* Find the next sample from the same stream, assuming there is one;
> + * this is used to calculate the duration below */
> +next_sample_id = film->current_sample + 1;
> +while (next_sample == NULL) {
> +if (next_sample_id >= film->sample_count) {
> +break;
> +}
>

Remove brackets, we don't put them if its a 1 line branch.



> +
> +next_sample = >sample_table[next_sample_id];
> +if (next_sample->stream != sample->stream) {
> +next_sample = NULL;
> +next_sample_id++;
> +}
> +}
> +
>  /* position the stream (will probably be there anyway) */
>  avio_seek(pb, sample->sample_offset, SEEK_SET);
>
> @@ -287,6 +304,10 @@ static int film_read_packet(AVFormatContext *s,
>  pkt->stream_index = sample->stream;
>  pkt->pts = sample->pts;
>  pkt->flags |= sample->keyframe;
> +pkt->dts = sample->pts;
> +if (next_sample != NULL) {
> +pkt->duration = next_sample->pts - sample->pts;
> +}
>

Same.



>
>  film->current_sample++;
>
> --
> 2.16.2
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] Sega FILM: set dts and duration when demuxing

[Michael Niedermayer]

On Wed, Mar 21, 2018 at 09:14:44AM -0700, mi...@brew.sh wrote:
> From: Misty De Meo 
> 
> ---
>  Changelog  |  1 +
>  libavformat/segafilm.c | 24 
>  2 files changed, 25 insertions(+)
> 
> diff --git a/Changelog b/Changelog
> index 7969b414c4..6b3b2bf7fe 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -47,6 +47,7 @@ version :
>  - native SBC encoder and decoder
>  - drmeter audio filter
>  - hapqa_extract bitstream filter
> +- segafilm: set timestamps when demuxing
>  
>  
>  version 3.4:
> diff --git a/libavformat/segafilm.c b/libavformat/segafilm.c
> index 11768823fc..2295fb3fa5 100644
> --- a/libavformat/segafilm.c
> +++ b/libavformat/segafilm.c
> @@ -270,6 +270,8 @@ static int film_read_packet(AVFormatContext *s,
>  FilmDemuxContext *film = s->priv_data;
>  AVIOContext *pb = s->pb;
>  film_sample *sample;
> +film_sample *next_sample = NULL;
> +int next_sample_id;
>  int ret = 0;
>  
>  if (film->current_sample >= film->sample_count)
> @@ -277,6 +279,21 @@ static int film_read_packet(AVFormatContext *s,
>  
>  sample = >sample_table[film->current_sample];
>  
> +/* Find the next sample from the same stream, assuming there is one;
> + * this is used to calculate the duration below */
> +next_sample_id = film->current_sample + 1;
> +while (next_sample == NULL) {
> +if (next_sample_id >= film->sample_count) {
> +break;
> +}
> +
> +next_sample = >sample_table[next_sample_id];
> +if (next_sample->stream != sample->stream) {
> +next_sample = NULL;
> +next_sample_id++;
> +}
> +}
> +
>  /* position the stream (will probably be there anyway) */
>  avio_seek(pb, sample->sample_offset, SEEK_SET);
>  
> @@ -287,6 +304,13 @@ static int film_read_packet(AVFormatContext *s,
>  pkt->stream_index = sample->stream;
>  pkt->pts = sample->pts;
>  pkt->flags |= sample->keyframe;

> +pkt->dts = sample->pts;

this may be redundant


> +pkt->size = sample->sample_size;

This is not correct, the packet has its size already set, if for some
reason like IO failures its different overriding this could crash the
code later


> +if (next_sample != NULL) {
> +pkt->duration = next_sample->pts - sample->pts;
> +} else {

> +pkt->duration = 0;
> +}

setting duration to "nothing"  should not be needed, this should be
already teh default

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel