mozilla :: #media

14 Mar 2017
01:14kamidphishkinetik: pulse in rust works in gecko \o/
01:14kamidphishkinetik: Need to run barrage of tests though
01:26kinetikkamidphish: neat
05:55ngmjf: when you get back, you can simply uncomment the --with-ccache line, it works fine. Interestingly a post clobber build is about 20% slower for me with ccache, which is a bigger than I would have thought.
06:19Ford_Prefectkamidphish: sweet!
06:54bwubechen: https://github.com/w3c/web-platform-tests/pull/4992#issuecomment-285343094
09:05jw_wangjya, ping
09:06jyajw_wang: pong
09:06jw_wangso how should I modify the test to pass and get an r+?
09:06jyaremove once(audiosb, 'updateend')
09:07jyait's the only thing wrong in that test.. I'm not sure why it's there
09:07jw_wangonly that?
09:07jyathere's no reason to count updateend.
09:07jyayes.. it's the only thing wrong with this test.
09:07jw_wangso I don't need the updateend handler
09:07jyano
09:07jw_wangand don't count the number of events.
09:07jyai don't understand how that test could have ever worked tbh
09:08jw_wangit is about the micro and macro tasks.
09:08jyaupdateend would have been fired due to the last "fetchAndLoad.bind(null, audiosb, 'aac51-48000-128000-"
09:09jw_wanghttp://searchfox.org/mozilla-central/rev/a5c2b278897272497e14a8481513fee34bbc7e2c/dom/media/mediasource/SourceBuffer.cpp#384
09:09jw_wangit queues 2 events in a row.
09:09jyaby then we would have had a minimum of one extra run of the JS event loop, updateend would have already been fired. the listener just can't get one.
09:10jyathat's irrelevant that it queues two event in a row
09:10jw_wangbut the 'update' handler and the 'updateend' *might* happen in the same cycle.
09:10jw_wangor *might not*
09:10jyasure... but why does this matter?
09:10jyauh?
09:10jyathey will always happen in the same cycle
09:11jw_wangif they happen on the same cycle, micro tasks will happen after them.
09:11jw_wangif not, micro tasks will happen in between them.
09:11jw_wangand promise run on micro tasks.
09:11jyawhy would it matter as far as JS is concerned
09:11jya?
09:11jyai'm not sure i understand where you're getting at
09:11jw_wangbecause the order of then() function will be different
09:12jya???
09:12jyahow can the order be different?
09:12jw_wangif they happen on the same cycle, it will be like 'update' handler -> 'updateend' handler -> then() functions.
09:12jyathey will always be in the same order, by order I mean task Y will always occur after task X
09:12jw_wangif not, it will be 'update' handler -> then() functions -> 'updateend' handler
09:12jyait doesn't matter if some micro, internal tasks occur in between
09:13jw_wangmost of the code of test_AudioChange_mp4.html run in then() functions.
09:14jyaI'm not sure where you're getting at, or what point you're trying to point out to be honest. that test is wrong, and to make it okay, the only thing needed is removing "promises.push(once(audiosb, 'updateend'));"
09:14jw_wangif they happen on the same cycle, it will be too late to register the 'updateend' handler in the then() function.
09:15jyawhile your conclusion is correct, how you get there isn't
09:15jw_wangno?
09:15Ford_Prefectkamidphish: where is this being used, btw?
09:15jyafetchAndLoad will load a buffer, and call appendBuffer, that will queue an "updateend"
09:15jyaand then resolve the promise
09:16jyaso when the next .then is run, the event has now been dispatched
09:16jw_wangfetchAndLoad depends on loadSegment which depends on the 'update' event.
09:16jw_wangso there are 2 events in concern here.
09:17jw_wangthe test case listens to 'updateend' event while fetchAndLoad listens to 'update' event.
09:17jyaanyway, if you want r+ remove "promises.push(once(audiosb, 'updateend'));" and that's it
09:18jyaI don't want to see a count of how many times the event has been fired, because I don't see how that adds any values. Every fetchAndLoad listend on the event already, if you come out of those, it's because the event has been fired. and num(update) == num(updateend)
09:18jyaI never understood why the MSE spec made provision for those two events.
09:18jyathey are always fire at the same time
09:19jyathey are both queued on the main thread, you can't have tasks happening in between
09:19jw_wangyou can have micro tasks happening in between.
09:21jyanot on the main thread you can't
09:22jw_wangI don't get it
09:22jyamain thread tasks that is
09:22jw_wangmicro tasks happen in between macro tasks.
09:23jyawell, let's just leave it at that then... I just don't want to see a test being explained being resolved for reason X when it failed for reason Y
09:23jyalistening for event updateend after a call to endOfStream was wrong
09:25jw_wangit is tricky because Quantum is trying to label runnables on the main thread which kinda change the order of events.
09:26jw_wangand things can get wrong if they make wrong assumptions about the order.
09:26jw_wanghttps://wiki.mozilla.org/Quantum/DOM
09:30kamidphishFord_Prefect: kinetik and I are working to remote pulse on Linux so the content sand box can be locked down. We're taking the opportunity to rewrite C in rust too.
09:31Ford_Prefectkamidphish: ah, remote in what sense?
09:32kamidphishFord_Prefect: remote in gecko means cross process. From content process to an audio server process.
09:39jhlinjya: couldn't find the code in your comment (always flush after drain). Is it in MFR?
09:40jyayes
09:41Ford_Prefectkamidphish: I see, interesting
09:41jyahmmm, looks like it's been removed
09:41Ford_PrefectI'd be curious to see how that turnso ut
09:41jyaI remember someone saying that it wasn't required anymore on android machine with a certain API
09:42jyawhen I linked to the android doc that stated that after a drain a decoder had to be flushed to accept new data
09:42jyalet me find that comment
09:43jyajhlin: you mentioned the same thing there: https://bugzilla.mozilla.org/show_bug.cgi?id=1299105#c17
09:43firebotBug 1299105 FIXED, jacheng@mozilla.com Video stutters when switching quality.
09:45jhlinjya: sorry I was asking your comment 13 in bug 1345545.
09:45firebothttps://bugzil.la/1345545 FIXED, jyavenard@mozilla.com SPS/PPS NAL not prepend to first frame.
09:46jyaIMHO, if the android decoder doesn't work after a drain until a flush we need to either 1- flush all the decoders after a drain, but more likely 2- chain a drain with flush on android
09:47jyabecause right now iit's going to be buggy under many tests
09:47jyafor now most of the MSE tests are disabled on android
09:47jyamochitest that is
09:49jyajhlin: oh I see... flush was always called after drain, because it was doing drain -> flush -> shutdown
09:49jyabut jw_wang has hidden the flush inside the shutdown method now
09:50jhlinjya: got it. Thanks!
09:51jyaso you're right, after a drain there's no flush on android
09:51jyaso back to doing what I wrote above 1) or 2)
09:51jyasounds important to me
09:52jhlinjya: in that case, I will do 2
09:52jyawouldn't be difficult in MFR
09:53jyamakes it sounds like there's a lot that will need to be uplifted with the remote data decoder :(
09:56jhlinjya: and about removing SPS checking for MSE , where do you think updating config should go?
09:56jyajhlin: why bother ?
09:56jyaplus it helps with broken MSE site that append data without adding new init segments
09:57jyagot contacted by google where Firefox plays ESPN content but chrome doesn't
09:57jhlinjya: I see.
09:57jyaso while it's not per spec, it allows to play some broken content
09:57jyaand it's still needed for MSE with AVC3
09:57jyaAVC3 is AVCC NAL, but the init segment doesn't contain the SPS / PPS
09:58jyainstead the SPS/PPS NAL are inband
09:58jyathat's what the BBC is using
09:59jhlinjya: thanks for the info. :)
11:22achronoppadenot: do you have more comments in #234? If you are ok I could merge
11:28achronopdo you have it in chrome ?
11:29achronop^ ignore, wrong chat
14:15sheppyjib|away jesup: can you create and manage an RTCPeerConnection in a worker?
14:16jibsheppy: no
14:16sheppyjib: didn't think so
14:17jibit's been discussed as an extension to the 1.0 webrtc spec, but there hasn't been enough interest in browsers
14:17sheppyIt would be sweet if you could; that'd be great for samples because then you could put the "remote peer" in a worker and have the context separation between local and remote that would help keep things clearer for example purposes.
14:19jibsheppy: I think the WG are looking for real-life use-cases first ;)
14:19sheppyjib: well of course.
14:19sheppyI'm just sayin' :)
14:19jibyeah that would be cool
14:20sheppyIf it had been something the real world use cases made sense to prioritize, it'd be great for my needs. :)
14:20sheppytis what tis
14:20jibdata channels might make sense to do on a worker
14:22jibsheppy: how about using iframes?
14:22sheppyjib: yeah, that's a good idea.
14:23sheppyI just don't want to confuse readers by having the two peers getting set up and managed in the same code stream. Took me ages to figure out how things work when looking at examples like that. :/
14:24jibsheppy: Did you see my new blog? It uses two tabs: https://blog.mozilla.org/webrtc/fiddle-of-the-week-await/
14:24sheppyjib: I didn't; looking now
14:27sheppyawait and async ar freaking awesome.
14:27sheppyare
14:27padenotachronop, I'm gonna pull in the latest cubeb fixes, after having looked at #234
14:28jibsheppy: I like 'em!
14:29jibpity the JS won't even load in 51
14:29sheppyjib: hey, the link "open this post in a second window" has preview=true on the URL. Is that intentional?
14:29jibno
14:29jibthanks, I'll fix that now
14:29sheppyI didn't think so.
14:30jibsheppy: updated. thanks!
14:31sheppyI still don't generally like arrow functions. I'm tend to find that the more you go to symbols instead of words, the more confusing code becomes to me.
14:32jibsheppy: I agree I overuse them in that fiddle
14:33jibI'll fix that
14:33sheppyDon't tweak it on my account. I suspect this is my problem more than anything. :)
14:37jibsheppy: No that's good feedback. I'll keep them for the function expressions, but I have at least one function declaration, and for those it's simpler to use plain old function I think
14:37sheppyI think so too, yeah.
14:40sheppyThe DTMF example I wrote vanished on me; I'm having to rewrite it from scratch. I'm very grumpy. :)
14:40Ford_Prefectjib: nice! :)
14:41achronoppadenot: I'll uplift part of bug 1345049, do you want to take any of your changes?
14:41firebothttps://bugzil.la/1345049 FIXED, nobody@mozilla.org Update cubeb from upstream to f07ee6d
14:41jibsheppy: FYI https://webrtc.github.io/samples/src/content/peerconnection/dtmf/ now works in Firefox
14:42padenotachronop, yeah we need windows fixes, it fixes two crashes
14:42achronopok, I'll take both
14:43padenotachronop, 1343972 and 1342389, for reference
14:43sheppyjib: did they fix the sample or did we stub the function they were checking to look for DTMF support? :)
14:43jibI fixed the sample - https://github.com/webrtc/samples/pull/873
14:43sheppyjib: good deal
14:44jibsheppy: That way our blog post could link to the webrtc samples - https://blog.mozilla.org/webrtc/dtmf-now-available-firefox/
14:45sheppyjib: makes sense
14:45jibWe still come out looking good by being first in my virew
14:45sheppyI agree.
14:45jibeven though they were first in a non-spec sense ;)
14:46sheppyI'm going to basically take this code, split the two connections, and just have an edit box you type a phone number into and a button to send the tones.
14:46padenotachronop, you're good with merging #234 ?
14:47achronopyeah, I am done, everything looks good for me
14:47jibsheppy: I have an async/await dtmf fiddle if you're interested - http://jsfiddle.net/jib1/6pfL7m3e/
14:47sheppyjib: that's the sexytimes right there. :)
14:48jibI wanted one that didn'
14:48jibt require permissions
14:48sheppyYeah.
14:48jibbut it got too odd, and in real life people always want DTMF ontop of voice I think
14:48sheppyYeah
14:49sheppyI think I'm going with the iframe idea for this one.
14:55Baspadenot: Can you tell me how canvas.captureStream works?
14:55jibsheppy: makes sense. From what I understand of workers, they might have ended up having slightly different async apis anyway
14:55sheppyjib: yeah, probably
14:55padenotBas, no, but I know I can ask pehrsons to tell you :-)
14:55Basworks for me :)
14:55pehrsonsyo
14:56pehrsonsBas: you've read the spec?
14:56Baspehrsons: Nah, and I don't really care about using it :P
14:56jyalol, I knew Bas was going to answer that
14:57BasI just want to know where it ties in to our video architecture code-wise
14:57* jya looking on how to log into a loaner
14:58pehrsonsBas: mkay, well, there's something that tells the canvas "hey I want your next frame", then on the next refresh driver tick, if there's been something drawn to the canvas, it'll grab a frame and push it out to all tracks that requested one
14:58pehrsonsit ties into video by feeding into VideoStreamTracks
15:00Baspehrsons: Does it ever touch our decoding code?
15:00pehrsonsBas: as in MediaDecoder?
15:00BasYeah
15:00pehrsonsno
15:01pehrsonsthis is all raw frames
15:01BasI have a patch to fix a bug in the decoder, but somehow, mysteriously I've been told it causes a permafail in the captureStream reftest :s
15:01pehrsonsoh I see
15:01pehrsonsthat sounds weird, like some kind of latent timing issue
15:02Baspehrsons: Timing shouldn't even be affected, I'm utterly confused.
15:02pehrsonspermafail you say, do you have it on treeherder?
15:02Baspehrsons: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=46090632bad356411c602147cadedb7822e3152e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
15:06padenotachronop, hrm it needs to be rebased manuall
15:06padenoty
15:06achronopthe PR?
15:07Baspehrsons: Oh no... I think I know.
15:09pehrsonsok, that's good, because I don't :-)
15:12pehrsonsthat one line in TextureD3D11.cpp?
15:14Baspehrsons: that's the one I suspect.
15:14pehrsonsI cannot imagine wmf being anywhere near
15:16Baspehrsons: Well, that line is hit at least.. I guess it makes sense that that would be broken.
15:20Baspehrsons: I guess there must exist code that syncs back to another device here, no idea how this works but I could see how I broke it..
15:23jyagrrrr. the more I read docs about mozharness and loaners, the more confuse I become
15:30Baspehrsons: Yep! I got this now, thanks! :)
15:37pehrsonscool!
15:56padenotachronop, hrm it conflicts
15:56padenotachronop, maybe I can just merge and squash and land
15:56HD|Laptophey all
15:57achronoppadenot: you can leave 234 out
15:57HD|Laptopit's me again, still trying to get reTurn working... this here is my Firefox webrtc log: http://paste.debian.net/919881/
15:57mreavypadenot, jesup - is the logging for the drift issue now in Nightly?
15:58padenotmreavy, I'm doing the uplift to get the patch we need as we speak
16:00mreavypadenot: so, in terms of timing, you land it on inbound now (or within the next hour) and then it merges a few hours later to m-c -- at that point, I could spin a new Nightly and start testing. Yes?
16:01padenotmreavy, no, because I haven't made sure it works
16:01padenotI can tell you in a few minutes
16:01padenotmaybe it's utterly broken, I don't want to give false hopes :-)
16:02mreavypadenot: does it need to be re-reviewed after you verify it works (or get it working)? Or does that depend on what you find?
16:02padenotmreavy, I don't think so, but I need to land a gecko patch at exactly the same time
16:02padenotshould be fine
16:05mreavypadenot: ok, let me know where we stand with this before you quit for the day. I'll add test calls (daily standups) to everyone's calendar once the logging we need is on inbound and sticking.
16:05padenotmreavy, hrm, we still need a review from billm
16:05mreavypadenot: ah, the logging touches IPC?
16:06padenotnot really, but some weird issue with the way gecko prefs work
16:06HD|Laptophmm that was caused by firewall overreach... well, works now, sorta
16:06HD|Laptophowever I get weeeeird lagging and stuttering
16:07mreavypadenot: we should give billm a heads up so he knows to look for the patch in his review queue when it's ready
16:07mreavyjesup: fyi ^^^
16:09achronoppadenot: I'm not sure what is conflicting but if it's #234 I will take care later (or tomorrow), no need to spend time on that
16:10padenotachronop, I think chunmin's base is before aggregated devices
16:11achronoppadenot: ok, just leave it as is, I know that part very well
16:11padenotok
16:12jesuppadenot: where's the review for billm? (bug)?
16:13padenotjesup, 1341238
16:13padenotwe can get another DOM peer to rs this
16:51padenotmreavy, jesup, got the review sorted, will land asap
16:53mreavypadenot: good, thanks.
17:17padenotmreavy, do we have a bug to track this OSX drift situation ?
17:29mjfjesup: ping
18:03padenotmreavy, ping
18:05padenotmreavy, I need to run, but I've updated bug 1341238 with the patches that need to land, and got reviews from ehsan and bz, and explain why we're rushing this
18:05firebothttps://bugzil.la/1341238 NEW, damien@zamaudio.com Patch to allow users to force a particular libcubeb audio backend
18:05padenotmreavy, anybody can land the four patches in this bug
18:06padenotmreavy, in particular, see comment 24: https://bugzilla.mozilla.org/show_bug.cgi?id=1341238#c24
18:06padenotjesup, ^
18:11jesuppadenot: I can land it
18:11padenotjesup. we need to wait for a try to go greenish
18:11drnobwc: ping
18:11jesupsure
18:11padenotjesup, links and stuff are in the bug, I really need to run
18:12jesupgo. thanks
18:12padenotthanks !
18:14bwcdrno: pong
18:15drnobwc: so for the a=setup topic I guess the most important question is: do we need to support non JSEP endpoints?
18:16bwcdrno: If the base spec says that an attribute has a default value, and the JSEP spec says that value is ok, I would say we should allow it.
18:18drnookay. so according to the discussion we had over which values are allowed it is acutally legit to use either actpass in an offer, or the value which has been negotiated in previous offer/answer
18:18bwcdrno: Ok, that's fine.
18:18drnoso I guess that means for the initial offer it can only be actpass, but for subsequent offers it could be actpass, active or passive
18:18bwcdrno: Right.
18:18mreavypadenot: thanks, enjoy your night. We'll get it landed and into Nightly asap.
18:20drnobwc: Im not sure that I like the idea of accepting missing a=setup, because it can lead to situations where one side assumes A and the other side assumes B
18:20bwcdrno: Well, the base spec says what the default value is.
18:20bwcdrno: If someone gets that wrong, they get it wrong.
18:20drnoI guess why this is so bad is that right now DTLS timeouts go undected. ICE connected, but A/V dont work
18:21bwcdrno: So who has messed up the default value stuff?
18:22drnobwc: well the original bug was that the other side did not include a=setup in a re-offer.
18:22drnobwc: and then we accounced A in our answer, but in fact transportlayerdtls assume B
18:23drnoIll try to solve that stuff in 1329028
18:23bwcdrno: Ok.
18:24drnobwc: Ill have another look
18:31mjfanyone used One Click Loaner?
18:58drnomjf: you mean from/for Taskcluster?
18:58mjfdrno: yep
18:58drnobwc: any reason we dont have a ValidateOffer() function in JsepSessionImpl?
18:59drnomjf: I have not so far, but I think jesup (^) might have
18:59mjfIm trying to track down a build issue on Windows, and I dont have (or want ;-)) and windows box here.
18:59bwcdrno: Guess we just haven't implemented any offer validation yet?
19:00drnobwc: okay. I need it now for the a=setup thing. So Ill add one :)
19:11jesupkinetik: ping
19:12ngjib: ping
19:12jibng: pong
19:21mjfjesup: ping
19:22jesupmjf: pong
19:22mjfjesup: have you used the one click loaner link from try?
19:23mjf(under job details)
19:24jesupmjf: no
19:24mjfjesup: Ill go ask in #developers. Thanks!
19:30kinetikjesup: pong
20:56geraldkinetik: Hi! Please don't start reviewing bug 1341483 yet, something landed in m-c that will require me to do an extensive rebase!
20:56firebothttps://bugzil.la/1341483 NEW, gsquelart@mozilla.com Report Rust demuxer issues as errors (or warnings if appropriate)
20:58kinetikgerald: cool, thanks for the warning
20:59gerald:-)
21:16drnobwc: we dont have a way to query the bundle master transport about something in Jsep, or?
21:19bwcdrno: I'm not sure I understand what you're asking.
21:20drnobwc: for the setup thing I would like to find the bundle master for a given msection, then get the transportlayer for that and query it what its current view on active vs passive is
21:21drnobecause we dont store the active vs passive no where (besides the transportlayerdtls knowing about its current role)
21:21drnobut the first part is what Im looking for right now: find the bundle master section for a given msection
21:22jesupkinetik: padenot's import of cubeb failed on Try - looks like an upstream patch to add a test is missing an #if (defined(_WIN32) || defined(__WIN32__)): https://treeherder.mozilla.org/logviewer.html#?job_id=83764774&repo=try&lineNumber=99130
21:22bwcSo JsepTransport has the level of the master in mBundleLevel, would that be enough to index into where you need?
21:22bwcdrno ^
21:23jesupkinetik: the new test was test_overload_callback.cpp
21:23drnobwc: good point. Ill check if that wroks for me. thx
21:23jesupkinetik: in https://hg.mozilla.org/try/rev/a213795ee9340db7da0e04634e1260ee2132fd9f
21:25kinetikjesup: okay
21:28jesupkinetik: how do you want to handle it? padenot had to leave for the day; I was going to land it if the try went green. Fix inline, then land an update that gets rid of the patch, or fix upstream and redo the import?
21:30kinetikjesup: just take test_overload_callback out of the gtest moz.build for now
21:30kinetikwe can fix it later
21:30kinetiki deliberately didn't add it during my earlier import because i don't know if it runs on our automation and didn't want to find out when we're in a rush to import
21:31jesupkinetik: Ok, will push new try with that
21:32kinetikjesup: okay, let me know if you need me to take over watching/landing it later
21:35jesupkinetik: I'll check in after dinner on it
21:37jesupjya: cpearce: https://pastebin.mozilla.org/8982046 -- is this expected in a debug build?
21:38jyajesup: you have PlatformDecoderModule:5 as debut option?
21:39jyaif you do, that enables the ffmpeg logs
21:39jesupjya: MOZ_LOG is empty
21:40jesupFedora 25 x64
21:40jyayou should ask cpeterson , he's the one who turned on the log with ffmpeg
21:40jyawhich version of libavocdec are you using?
21:41jesupcpeterson: ping ^
21:41cpeterson?
21:41cpetersonwhich log is that?
21:41jesup /usr/lib64/libavcodec.so.56.26.100
21:43cpetersoncpearce? ^ I don't think I enabled ffmpeg logs.
21:44cpearceI don't recall ever touching the ffmpeg code.
21:44jesupIs there anything else I shoudl check?
21:44jyahttps://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/ffmpeg/FFmpegLibWrapper.cpp#144
21:44cpearceThose logs log benign to me.
21:44jyaso yes, a debug build will be very verbose
21:44jyacpeterson: I'm fairly certain you did :) but I could be wrong
21:45jesupjya: that line was landed in Bug 1240995
21:45firebothttps://bugzil.la/1240995 FIXED, jyavenard@mozilla.com Use common function accessor between FFmpeg and FFVPX to call FFmpeg API
21:45jyayes, but that's just rewriting what was already there
21:45jesupah
21:46jesupI wanted to know if we expected to do that sort of logging in debug builds (regardless of MOZ_LOG/etc settings)
21:47jyajesup: it seems you do
21:47jyadoing av_log_set_level(AV_LOG_DEBUG); is a global settings
21:47jyait changes a static variable
21:47jesupsecond question is "is this a good idea" or "is there a better idea", which I'll let you worry about. :-)
21:48jyaproblem there is that the initialisation of ffmpeg lib is done once
21:48jyai guess it would be nicer to check if the MOZ_LOG is set
21:48jyaand only call that then
21:48jesupffmpeg:4?
21:49jesupthen you could turn it on in release/opt builds
21:49jesupwhich might be a plus for debugging problems in the field
21:54jyawe use a generic PlatformDecoderModule already
21:54jyajesup: if you could open a bug for this, would be appreciated
21:59jesupsure
22:09mjfabr: ping
22:10abrpong
22:54kamidphishyay, test_playback.html times out. :-(
23:22kamidphishOh, so it's going to be one of *those* days....
23:22kamidphishhttps://irccloud.mozilla.com/pastebin/TrDjAXNM/
15 Mar 2017
No messages
   
Last message: 41 days and 5 hours ago