Re: [FFmpeg-devel] [PATCH v2] avformat/movenc : Don't write sidx for empty urls
On 12/1/18 12:01 AM, Michael Niedermayer wrote: > On Fri, Nov 30, 2018 at 05:52:35AM +, Jeyapal, Karthick wrote: > > > > On 11/29/18 11:08 PM, Michael Niedermayer wrote: > > > On Wed, Nov 28, 2018 at 09:45:24PM +0530, Karthick J wrote: > > >> When movenc is used by other segmenting muxers such as dashenc, url > > >> field is always empty. > > >> In such cases it is better to not write sidx, instead of throwing errors. > > >> --- > > >> libavformat/movenc.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > > > > > please correct me if i misunderstand but > > > dashenc enables global sidx, skiping writing the sidx then always seems a > > > bit > > > odd > > Your understanding is correct. > > But the motive of dashenc is to disable writing of sidx atom for every moof > > atom (Let's call this as local sidx for the sake of discussion). > > We wanted to disable local sidx as it was adding significant bitrate > > overhead for chunked streaming(where each frame is a moof). > > To disable local sidx we enabled global sidx. And global sidx was not > > writing any sidx at all from dashenc due to lack of url. > > This behavior of not writing any sidx at all is also acceptable for dash. > > But we just wanted to remove the error, and hence this patch. > > Maybe one could add a new option no_sidx in movenc for this functionality. > > But since global_sidx was already achieving the same functionality new > > option seemed a bit redundant. > > I have no real oppinion on how to solve it, adding a more specific option > could be done. > but enabling global_sidx and then not doing it silently seems quite hackish > as the intended behavior. > I mean please implement this cleanly, whichever way makes most sense. Thanks for your comments. I have submitted a patch with a specific option to disable sidx as you suggested. http://ffmpeg.org/pipermail/ffmpeg-devel/2018-December/237121.html regards, Karthick > > thanks > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Observe your enemies, for they first find out your faults. -- Antisthenes ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/movenc : Don't write sidx for empty urls
On Fri, Nov 30, 2018 at 05:52:35AM +, Jeyapal, Karthick wrote: > > On 11/29/18 11:08 PM, Michael Niedermayer wrote: > > On Wed, Nov 28, 2018 at 09:45:24PM +0530, Karthick J wrote: > >> When movenc is used by other segmenting muxers such as dashenc, url field > >> is always empty. > >> In such cases it is better to not write sidx, instead of throwing errors. > >> --- > >> libavformat/movenc.c | 3 +++ > >> 1 file changed, 3 insertions(+) > > > > please correct me if i misunderstand but > > dashenc enables global sidx, skiping writing the sidx then always seems a > > bit > > odd > Your understanding is correct. > But the motive of dashenc is to disable writing of sidx atom for every moof > atom (Let's call this as local sidx for the sake of discussion). > We wanted to disable local sidx as it was adding significant bitrate overhead > for chunked streaming(where each frame is a moof). > To disable local sidx we enabled global sidx. And global sidx was not writing > any sidx at all from dashenc due to lack of url. > This behavior of not writing any sidx at all is also acceptable for dash. But > we just wanted to remove the error, and hence this patch. > Maybe one could add a new option no_sidx in movenc for this functionality. > But since global_sidx was already achieving the same functionality new option > seemed a bit redundant. I have no real oppinion on how to solve it, adding a more specific option could be done. but enabling global_sidx and then not doing it silently seems quite hackish as the intended behavior. I mean please implement this cleanly, whichever way makes most sense. thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/movenc : Don't write sidx for empty urls
On 11/29/18 11:08 PM, Michael Niedermayer wrote: > On Wed, Nov 28, 2018 at 09:45:24PM +0530, Karthick J wrote: >> When movenc is used by other segmenting muxers such as dashenc, url field is >> always empty. >> In such cases it is better to not write sidx, instead of throwing errors. >> --- >> libavformat/movenc.c | 3 +++ >> 1 file changed, 3 insertions(+) > > please correct me if i misunderstand but > dashenc enables global sidx, skiping writing the sidx then always seems a bit > odd Your understanding is correct. But the motive of dashenc is to disable writing of sidx atom for every moof atom (Let's call this as local sidx for the sake of discussion). We wanted to disable local sidx as it was adding significant bitrate overhead for chunked streaming(where each frame is a moof). To disable local sidx we enabled global sidx. And global sidx was not writing any sidx at all from dashenc due to lack of url. This behavior of not writing any sidx at all is also acceptable for dash. But we just wanted to remove the error, and hence this patch. Maybe one could add a new option no_sidx in movenc for this functionality. But since global_sidx was already achieving the same functionality new option seemed a bit redundant. > > [...] > > ___ > 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 v2] avformat/movenc : Don't write sidx for empty urls
On Wed, Nov 28, 2018 at 09:45:24PM +0530, Karthick J wrote: > When movenc is used by other segmenting muxers such as dashenc, url field is > always empty. > In such cases it is better to not write sidx, instead of throwing errors. > --- > libavformat/movenc.c | 3 +++ > 1 file changed, 3 insertions(+) please correct me if i misunderstand but dashenc enables global sidx, skiping writing the sidx then always seems a bit odd [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v2] avformat/movenc : Don't write sidx for empty urls
When movenc is used by other segmenting muxers such as dashenc, url field is
always empty.
In such cases it is better to not write sidx, instead of throwing errors.
---
libavformat/movenc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 6dab5193b0..d47ecc65ca 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6706,6 +6706,9 @@ static int mov_write_trailer(AVFormatContext *s)
mov->tracks[i].data_offset = 0;
if (mov->flags & FF_MOV_FLAG_GLOBAL_SIDX) {
int64_t end;
+// If url is an empty string("") don't write sidx atom.
+if (s->url[0] == '\0')
+return 0;
av_log(s, AV_LOG_INFO, "Starting second pass: inserting sidx
atoms\n");
res = shift_data(s);
if (res < 0)
--
2.17.1 (Apple Git-112)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
