On Fri, Apr 27, 2018 at 04:33:27AM +0000, Jeyapal, Karthick wrote:
>
>
> On 4/27/18 4:26 AM, Michael Niedermayer wrote:
> > On Fri, Apr 27, 2018 at 12:35:42AM +0300, Jan Ekström wrote:
> >> On Thu, Apr 26, 2018 at 12:00 PM, Jeyapal, Karthick <kjeya...@akamai.com>
> >> wrote:
> >>>
> >>>
> >>> On 4/23/18 11:40 AM, Karthick J wrote:
> >>>> From: Karthick Jeyapal <kjeya...@akamai.com>
> >>>>
> >>>> There is a separate muxer(webmdashenc.c) for supporting VP9+webm output
> >>>> in DASH.
> >>>> Hence in this muxer we will focus on supporting VP9 in MP4
> >>>> Have verified playout support of VP9+MP4 in Chrome and Firefox.
> >>>> ---
> >>>> libavformat/dashenc.c | 3 +--
> >>>> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> >>>> index a5f58d4..211ef23 100644
> >>>> --- a/libavformat/dashenc.c
> >>>> +++ b/libavformat/dashenc.c
> >>>> @@ -959,11 +959,10 @@ static int dash_init(AVFormatContext *s)
> >>>> if (!ctx)
> >>>> return AVERROR(ENOMEM);
> >>>>
> >>>> - // choose muxer based on codec: webm for VP8/9 and opus, mp4
> >>>> otherwise
> >>>> + // choose muxer based on codec: webm for VP8 and opus, mp4
> >>>> otherwise
> >>>> // note: os->format_name is also used as part of the mimetype
> >>>> of the
> >>>> // representation, e.g. video/<format_name>
> >>>> if (s->streams[i]->codecpar->codec_id == AV_CODEC_ID_VP8 ||
> >>>> - s->streams[i]->codecpar->codec_id == AV_CODEC_ID_VP9 ||
> >>>> s->streams[i]->codecpar->codec_id == AV_CODEC_ID_OPUS ||
> >>>> s->streams[i]->codecpar->codec_id == AV_CODEC_ID_VORBIS) {
> >>>> snprintf(os->format_name, sizeof(os->format_name), "webm");
> >>>
> >>> Pushed the patchset
> >>>
> >>
> >> Hi,
> >>
> >> First of all, I would prefer the patch to actually mention what it is
> >> doing. It is removing the webm override from the muxer for VP9. There
> >> is as far as I know no option to switch between modes in current git,
> >> so the commit message is blindly misleading at best and just plain
> >> trying to look harmless (in order to get a free pass) if taken the
> >> wrong way. Not saying you meant it that way, but the commit message
> >> does not say what it does as far as I can see.
> >>
> >> Also the patch does not mention the reason why it is doing this other
> >> than the fact that there's a separate webm DASH muxer. That is true,
> >> but the real reason you were switching this default is because the
> >> WebM mode does not work. Now, fixing segfaults is good. And, for the
> >> record, I actually agree with the change since with the profile string
> >> it works in dash.js on various browsers (Firefox, Chromium, Edge).
> >>
> >> But begesus... If it is done like this you might as well be honest and
> >> just remove the WebM mode! Because right now you left it there to
> >> segfault for VP8, Opus, Vorbis. Another alternative would have been to
> >> apply the small change to Rodger Combs's patch
> >> (https://patchwork.ffmpeg.org/patch/7984/), which you even commented
> >> on before! Maybe it still doesn't work in browsers, but at least it
> >> would have gotten to that point.
> >>
> >> Really, I am thankful that you are contributing, but I really do not
> >> want to see things like these after long days of work when I have not
> >> noticed or wasn't able to write a long reply. You waited for two days,
> >> and pushed without reviews even though I had shown interest in the
> >> patch in its first iteration. If you are interested in getting quick
> >> comments from someone (including me when I am awake and available),
> >> please join the IRC channel #ffmpeg-devel if only possible. Even if it
> >> is just for pokes and links to patchwork towards someone who has shown
> >> interest to related patches before. I try as much as possible to poke
> >> relevant people when I post patches, and I would prefer it if others
> >> would do that as well. We're not perfect and issues can and do go
> >> through peoples' eyes (esp. if the change set is of the larger size
> >> issues tend to get hidden in plain sight, unfortunately), but let's
> >> try to make this work.
> >>
> >> Best regards,
> >> Jan
> >>
> >> P.S. I am sorry if my way of speech came out bad, it is just past
> >> midnight here and I was trying to get a reply to this written after
> >> noticing this mail.
> >
> > I can confirm that there still is a segfault occuring
> > ./ffmpeg -i ~/fatesamples/fate/fate-suite/vp8/RRSF49-short.webm -c copy
> > -b:v 2M out.mpd
> >
> > Jeyapal, do you have time to look at these segfaults?
> > ffmpeg should not segfault!
> > Its also very important that no invalid files are generated (just saying so
> > we dont end with a quick segfault fix that then produces bad files)
> We do have a fix from Jan for segfault issue.
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-April/229082.html
> I will test it and push it after 3 days. > I will think it affects 4.0 branch as well. Should I push that patch to 4.0 > as well? yes if you are sure that its better with the patch for 4.0 thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
