Re: [FFmpeg-devel] [PATCH 2/3] lavf/segment: slight refactor to seg_write_packet

2016-03-25 Thread Rodger Combs 

> On Mar 25, 2016, at 09:45, Stefano Sabatini  wrote:
> 
> On date Sunday 2016-03-06 20:49:23 -0600, Rodger Combs encoded:
>> This reduces some code duplication, and ensures that cur_entry.last_duration 
>> is
>> always set.
>> ---
>> libavformat/segment.c | 12 +++-
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/libavformat/segment.c b/libavformat/segment.c
>> index 818c0c9..6ad991f 100644
>> --- a/libavformat/segment.c
>> +++ b/libavformat/segment.c
>> @@ -824,11 +824,13 @@ static int seg_write_packet(AVFormatContext *s, 
>> AVPacket *pkt)
>> seg->cur_entry.index = seg->segment_idx + seg->segment_idx_wrap * 
>> seg->segment_idx_wrap_nb;
>> seg->cur_entry.start_time = (double)pkt->pts * av_q2d(st->time_base);
>> seg->cur_entry.start_pts = av_rescale_q(pkt->pts, st->time_base, 
>> AV_TIME_BASE_Q);
> 
>> -seg->cur_entry.end_time = seg->cur_entry.start_time +
>> -pkt->pts != AV_NOPTS_VALUE ? (double)(pkt->pts + pkt->duration) 
>> * av_q2d(st->time_base) : 0;
> 
> Unrelated, this looks wrong

Reading over the deleted code again, this confuses me a lot. In the 
segment-breaking case, if pkt->pts != AV_NOPTS_VALUE, we previously effectively 
set `seg->cur_entry.end_time = (double)(pkt->pts * 2 + pkt->duration) * 
av_q2d(st->time_base)`, if I understand correctly, and that doesn't seem to 
make any sense.

> 
>> -} else if (pkt->pts != AV_NOPTS_VALUE && pkt->stream_index == 
>> seg->reference_stream_index) {
>> -seg->cur_entry.end_time =
>> -FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + 
>> pkt->duration) * av_q2d(st->time_base));
>> +seg->cur_entry.end_time = seg->cur_entry.start_time;
>> +}
>> +
>> +if (pkt->stream_index == seg->reference_stream_index) {
>> +if (pkt->pts != AV_NOPTS_VALUE)
>> +seg->cur_entry.end_time =
>> +FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + 
>> pkt->duration) * av_q2d(st->time_base));
>> seg->cur_entry.last_duration = pkt->duration;
> 
> LGTM (and fixes the defect), thanks.
> -- 
> FFmpeg = Formidable and Fascinating Multimedia Powered Elegant Gnome
> ___
> 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 2/3] lavf/segment: slight refactor to seg_write_packet

2016-03-25 Thread Stefano Sabatini 
On date Sunday 2016-03-06 20:49:23 -0600, Rodger Combs encoded:
> This reduces some code duplication, and ensures that cur_entry.last_duration 
> is
> always set.
> ---
>  libavformat/segment.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/segment.c b/libavformat/segment.c
> index 818c0c9..6ad991f 100644
> --- a/libavformat/segment.c
> +++ b/libavformat/segment.c
> @@ -824,11 +824,13 @@ static int seg_write_packet(AVFormatContext *s, 
> AVPacket *pkt)
>  seg->cur_entry.index = seg->segment_idx + seg->segment_idx_wrap * 
> seg->segment_idx_wrap_nb;
>  seg->cur_entry.start_time = (double)pkt->pts * av_q2d(st->time_base);
>  seg->cur_entry.start_pts = av_rescale_q(pkt->pts, st->time_base, 
> AV_TIME_BASE_Q);

> -seg->cur_entry.end_time = seg->cur_entry.start_time +
> -pkt->pts != AV_NOPTS_VALUE ? (double)(pkt->pts + pkt->duration) 
> * av_q2d(st->time_base) : 0;

Unrelated, this looks wrong

> -} else if (pkt->pts != AV_NOPTS_VALUE && pkt->stream_index == 
> seg->reference_stream_index) {
> -seg->cur_entry.end_time =
> -FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + 
> pkt->duration) * av_q2d(st->time_base));
> +seg->cur_entry.end_time = seg->cur_entry.start_time;
> +}
> +
> +if (pkt->stream_index == seg->reference_stream_index) {
> +if (pkt->pts != AV_NOPTS_VALUE)
> +seg->cur_entry.end_time =
> +FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + 
> pkt->duration) * av_q2d(st->time_base));
>  seg->cur_entry.last_duration = pkt->duration;

LGTM (and fixes the defect), thanks.
-- 
FFmpeg = Formidable and Fascinating Multimedia Powered Elegant Gnome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/3] lavf/segment: slight refactor to seg_write_packet

2016-03-06 Thread Rodger Combs 
This reduces some code duplication, and ensures that cur_entry.last_duration is
always set.
---
 libavformat/segment.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libavformat/segment.c b/libavformat/segment.c
index 818c0c9..6ad991f 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -824,11 +824,13 @@ static int seg_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 seg->cur_entry.index = seg->segment_idx + seg->segment_idx_wrap * 
seg->segment_idx_wrap_nb;
 seg->cur_entry.start_time = (double)pkt->pts * av_q2d(st->time_base);
 seg->cur_entry.start_pts = av_rescale_q(pkt->pts, st->time_base, 
AV_TIME_BASE_Q);
-seg->cur_entry.end_time = seg->cur_entry.start_time +
-pkt->pts != AV_NOPTS_VALUE ? (double)(pkt->pts + pkt->duration) * 
av_q2d(st->time_base) : 0;
-} else if (pkt->pts != AV_NOPTS_VALUE && pkt->stream_index == 
seg->reference_stream_index) {
-seg->cur_entry.end_time =
-FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + pkt->duration) 
* av_q2d(st->time_base));
+seg->cur_entry.end_time = seg->cur_entry.start_time;
+}
+
+if (pkt->stream_index == seg->reference_stream_index) {
+if (pkt->pts != AV_NOPTS_VALUE)
+seg->cur_entry.end_time =
+FFMAX(seg->cur_entry.end_time, (double)(pkt->pts + 
pkt->duration) * av_q2d(st->time_base));
 seg->cur_entry.last_duration = pkt->duration;
 }
 
-- 
2.7.2

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