1033144 - implement cuechange event

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Ralph Giles (:rillian)]

When the list of active cues changes as a result of running the 'time marches on' algorithm, a TextTrack object should queue a 'cuechange' event.

http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#event-media-cuechange

[Carrying over implementation work from bug 996331]

Comment 1

[Ralph Giles (:rillian)]

Any progress on the quick implementation you mentioned, Rick?

Comment 2

[Rick Eyre (:reyre)]

Nope, not yet. I didn't start immediately since I saw your patch for removal. I should be able to find sometime this week to work on it though, shouldn't be too hard.

Comment 3

[Ralph Giles (:rillian)]

Ok. The removal patch was quick and an obvious improvement so I went ahead and did it. If we can get a small patch to dispatch the event working in nightly we can try to approve it for aurora as well.

Comment 4

[Rick Eyre (:reyre)]

Sounds good. I'll take a look at it tomorrow or Friday.

Comment 5

[Rick Eyre (:reyre)]

Attachment: Part 1: Dispatch 'cuechange' event on TextTracks.

Comment 6

[Rick Eyre (:reyre)]

Attachment: Part 2: Add tests for TextTrack cuechange event.

Comment 7

[Rick Eyre (:reyre)]

Work so far, haven't gotten the tests to work yet. Will work more on it Wednesday.

Comment 8

[Rick Eyre (:reyre)]

Attachment: Part 2: Add tests for TextTrack cuechange event.

Comment 9

[Rick Eyre (:reyre)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=0b2125af10cc

Comment 10

[Ralph Giles (:rillian)]

Comment on attachment 8452078 [details] [diff] [review]
Part 1: Dispatch 'cuechange' event on TextTracks.

Review of attachment 8452078 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comments addressed. Thanks for working on this!

::: content/media/TextTrack.cpp
@@ +158,5 @@
>    if (mDirty) {
>      mCuePos = 0;
>      mDirty = false;
>      mActiveCueList->RemoveAll();
> +    hasChanged = true;

Hmm. I guess you need this because you're removing all Cues and won't necessarily add one back. Is it correct to fire the event if you end up adding back exactly the same cues?

@@ +163,3 @@
>    }
>  
> +

I don't think you need this extra blank line.

Comment 11

[Ralph Giles (:rillian)]

Comment on attachment 8460690 [details] [diff] [review]
Part 2: Add tests for TextTrack cuechange event.

Review of attachment 8460690 [details] [diff] [review]:
-----------------------------------------------------------------

The general approach looks good. r- for issues.

::: content/media/test/test_texttrackevents.html
@@ +32,5 @@
> +      if (trackElement.readyState == 1) {
> +        return setTimeout(run_tests, 0);
> +      }
> +      is(trackElement.readyState, 2, "Track::ReadyState should be set to LOADED.");
> +      is(trackElement.track.oncuechange, null, "Track::OnCueChange should exist.");

By comparing with null, you're verifying it's not undefined, which is what you get when the event handler isn't present, and you're verifying that no handler is set, which isn't what the failure message says.

This is brittle, because:

null === undefined => false
null ==  undefined => true

And in fact, it looks like is() uses == so this won't verify the presence of the attribute.

Better to ok(foo === null) with a comment explaining the difference, or find a more clear way to verify this. I haven't figured out a better way. hasOwnProperty is close, but our implementation in in the prototype so that doesn't work. :/

@@ +42,5 @@
> +        // 12 all together. Since we're listening for the events on the TextTrack
> +        // as well as the TrackElement that should be 24 total, however, the last
> +        // exit event doesn't fire for both since it's end time is too close to
> +        // the end of the video for the fire time update event to catch it. Hence,
> +        // 24 - 2 == 22.

I'm worried this will cause intermittent failures later. Is the count reliable if you use a longer video, like vp9cake.webm? Should we be doing an extra check when playback finishes to make sure the final cues stop showing?

Comment 12

[Rick Eyre (:reyre)]

(In reply to Ralph Giles (:rillian) from comment #10)

Thanks for review Ralph!

> ::: content/media/TextTrack.cpp
> @@ +158,5 @@
> >    if (mDirty) {
> >      mCuePos = 0;
> >      mDirty = false;
> >      mActiveCueList->RemoveAll();
> > +    hasChanged = true;
> 
> Hmm. I guess you need this because you're removing all Cues and won't
> necessarily add one back. Is it correct to fire the event if you end up
> adding back exactly the same cues?

If that happened I don't think it would be since the track hasn't actually changed... but I'm having a hard time thinking of a case where that would happen. That would mean that the cue end time <= current play back time and its start time is also >= current play back time and that its already been added to the list, which it wouldn't be since its start time has just become valid for the current play back time.

Comment 13

[Rick Eyre (:reyre)]

(In reply to Ralph Giles (:rillian) from comment #11)

> ::: content/media/test/test_texttrackevents.html
> @@ +32,5 @@
> > +      if (trackElement.readyState == 1) {
> > +        return setTimeout(run_tests, 0);
> > +      }
> > +      is(trackElement.readyState, 2, "Track::ReadyState should be set to LOADED.");
> > +      is(trackElement.track.oncuechange, null, "Track::OnCueChange should exist.");
> 
> By comparing with null, you're verifying it's not undefined, which is what
> you get when the event handler isn't present, and you're verifying that no
> handler is set, which isn't what the failure message says.
> 
> This is brittle, because:
> 
> null === undefined => false
> null ==  undefined => true
> 
> And in fact, it looks like is() uses == so this won't verify the presence of
> the attribute.
> 
> Better to ok(foo === null) with a comment explaining the difference, or find
> a more clear way to verify this. I haven't figured out a better way.
> hasOwnProperty is close, but our implementation in in the prototype so that
> doesn't work. :/

Ah, good catch. I'm thinking it might be better to do okay('oncuechange' in track) as the in operator also checks the object's prototype.

> @@ +42,5 @@
> > +        // 12 all together. Since we're listening for the events on the TextTrack
> > +        // as well as the TrackElement that should be 24 total, however, the last
> > +        // exit event doesn't fire for both since it's end time is too close to
> > +        // the end of the video for the fire time update event to catch it. Hence,
> > +        // 24 - 2 == 22.
> 
> I'm worried this will cause intermittent failures later. Is the count
> reliable if you use a longer video, like vp9cake.webm? Should we be doing an
> extra check when playback finishes to make sure the final cues stop showing?

I think it would be more reliable if we used a longer video and stopped showing cues at least 1 second before the video ended. I think that would give us more than enough time to have a fire time update event trigger oncuechange. I'll update the patch to do that.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8452078 [details] [diff] [review]
Part 1: Dispatch 'cuechange' event on TextTracks.

Doesn't the spec say to queue a task to fire this event?

Comment 15

[Boris Zbarsky [:bzbarsky]]

And if so, can we add a test that would catch the issue?

Comment 16

[Ralph Giles (:rillian)]

(In reply to Boris Zbarsky [:bz] from comment #14)

> Doesn't the spec say to queue a task to fire this event?

So it does. Rick, I asked on irc, and smaug suggested

> (new AsyncEventDispatcher())->PostDOMEvent();

There's an already-implemented example of 'queue a task to fire a simple event' here: http://dxr.mozilla.org/mozilla-central/source/content/base/src/Element.cpp#2853

As for writing a test, I'm not sure how to do that. Apparently DispatchTrustedEvent() will call any registered handlers _before_ returning from whatever triggered oncuechange, so you could register one handler before and one after and check that both get it, but you'd have to find some reliable way to make the event fire inside a call.

Comment 17

[Rick Eyre (:reyre)]

(In reply to Boris Zbarsky [:bz] from comment #14)

> Doesn't the spec say to queue a task to fire this event?

Yup, you're right.

(In reply to Ralph Giles (:rillian) from comment #16)

> So it does. Rick, I asked on irc, and smaug suggested
> 
> > (new AsyncEventDispatcher())->PostDOMEvent();
> 
> There's an already-implemented example of 'queue a task to fire a simple
> event' here:
> http://dxr.mozilla.org/mozilla-central/source/content/base/src/Element.
> cpp#2853

Great, thanks for the help. I'll update the patch to do it that way.

> As for writing a test, I'm not sure how to do that. Apparently
> DispatchTrustedEvent() will call any registered handlers _before_ returning
> from whatever triggered oncuechange, so you could register one handler
> before and one after and check that both get it, but you'd have to find some
> reliable way to make the event fire inside a call.

Yeah, that might be a bit tricky. I'll see if I can get it done.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8452078 [details] [diff] [review]
Part 1: Dispatch 'cuechange' event on TextTracks.

OK.  r=me on the rest, but I'd like to see the patch updated with the event async.

Comment 19

[Rick Eyre (:reyre)]

Attachment: Part 1: Dispatch 'cuechange' event on TextTracks r=rillian, r=bz

Thanks for review Boris.

Carrying forward r=rillian,bz.

I've updated the event to be on the TextTrack to be fired asynchronously using the method
that Ralph suggested. I've also removed the setting of 'hasChanged' to true when the TextTrack
became dirty. This is because if the TextTrack gets dirty it's going to either remove cues
or add them anyways and set the hasChanged flag anyways.

Comment 20

[Rick Eyre (:reyre)]

Attachment: Part 2: Add tests for TextTrack cuechange event r=rillian

I've updated the test to use vp9cake.webm to give the events a little more time. I've
also started using a different VTT file that is more sequential so we can be sure that
the events will be regular and will have a single event for each enter/exit on a cue.

I'm not quite sure how to test the async here. Or even if it's possible. Can you explain
a bit more about the approach you were thinking Ralph?

Comment 21

[Ralph Giles (:rillian)]

Sorry(In reply to Rick Eyre (:reyre) from comment #19)

> that Ralph suggested. I've also removed the setting of 'hasChanged' to true
> when the TextTrack
> became dirty. This is because if the TextTrack gets dirty it's going to
> either remove cues
> or add them anyways and set the hasChanged flag anyways.

I don't think this is correct. I was worried about removing all cues, then adding the same ones back firing cuechanged. The more common case is removing them all and adding different ones, but you should still fire cuechanged if you remove them all and don't end up adding any.

Sorry for the review delay on the test patch. I'll get to it tomorrow.

Comment 22

[Ralph Giles (:rillian)]

Comment on attachment 8462303 [details] [diff] [review]
Part 2: Add tests for TextTrack cuechange event r=rillian

Review of attachment 8462303 [details] [diff] [review]:
-----------------------------------------------------------------

This is better, but I'd like to see it again with comments addressed.

Re testing the async dispatch, my understanding is if you do:

> var e = new Element();
> var count = 0;
> e.addEventListener('onsomevent', function foo() { ++count; });
> e.doCall();
> e.addEventListner('onsomeevent', function bar() { ++count; });

then if doCall() dispatches 'someevent' synchronously, count will be 1, and if it dispatches it asynchronously count will be 2.

So this gives you a way to test when the event is dispatched: if it's inside doCall() only foo() will be called. If doCall() instead queues a task, it won't be dispatched until the event loop spins again, e.g. after the script finishes executing, and at that time foo() and bar() will both be registered.

However, I can't think of any equivalent to doCall() for TextTracks which wouldn't already by async. That is, seeking updates the playback position asynchronously; addCue is synchronous, but it looks like cuechange only fires inside 'time marches on' which is triggered asynchronously. So any test like I've just described would always pass, because there's no way to trigger cuechange that doesn't already queue a task long before it gets to actually updating the active cue list.

::: content/media/test/test_texttrackevents.html
@@ +14,5 @@
> +<pre id="test">
> +<script class="testbody" type="text/javascript">
> +SimpleTest.waitForExplicitFinish();
> +SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true],
> +                                   ["media.webvtt.regions.enabled", true]]},

Don't need region support. :)

@@ +42,5 @@
> +        // for when it is activated/deactivated (6 events total), and we are
> +        // listening on both the track and track element, 12 total.
> +        if (++count === 12) {
> +          ok(true, "cuechange should have been fired 12 times.");
> +          SimpleTest.finish();

This is better, but won't detect if extra events are fired. Increment count here, and attach a separate 'onended' listener to video which verifies the count is 12 and finishes the test. That's also less likely to time out if fewer than 12 events fire.

Comment 23

[Silvia Pfeiffer]

While you're at it, you should also check the onload event, which isn't raised either.
Example: http://html5videoguide.net/NEW/CH4/code_c4_8.html
Works in Chrome 36.
Spec: http://www.w3.org/html/wg/drafts/html/master/embedded-content.html#event-media-cuechange

Comment 24

[Silvia Pfeiffer]

Sorry, the example has moved to http://html5videoguide.net/NEW/CH4/code_c4_9.html

Comment 25

[Rick Eyre (:reyre)]

Sorry Silvia, I don't see an 'onload' event in the link you posted?

Comment 26

[Rick Eyre (:reyre)]

Sorry for the _super_ late reply Ralph. Life got busy for a while and I didn't have anytime to finish this patch.

(In reply to Ralph Giles (:rillian) from comment #21)

> I don't think this is correct. I was worried about removing all cues, then
> adding the same ones back firing cuechanged. The more common case is
> removing them all and adding different ones, but you should still fire
> cuechanged if you remove them all and don't end up adding any.

I'm not quite sure what the best approach here would be to solve the case where we remove all the cues and then add the same ones back. My first idea would be to just track what cues were active at time of removal and then check that against the ones that are added back. It might be the simplest approach. Do you have any suggestions?

Comment 27

[Silvia Pfeiffer]

http://html5videoguide.net/NEW/CH4/code_c4_10.html is the right demo now.
track.onload fires in Chrome.
But I can't find it in the spec any more...

Comment 28

[Rick Eyre (:reyre)]

(In reply to Rick Eyre (:reyre) from comment #26)

> I'm not quite sure what the best approach here would be to solve the case
> where we remove all the cues and then add the same ones back. My first idea
> would be to just track what cues were active at time of removal and then
> check that against the ones that are added back. It might be the simplest
> approach. Do you have any suggestions?

Looking more at this I think I'm just going to refactor the UpdateActiveCueList function to make it easier to keep track of things like this. There are some improvements we can make here I think...

Comment 29

[daniel.weinberger]

I just wanted to check the status of this issue as it is still not resolved. Even as of Firefox 42 Beta I can't get a cuechange event. As MSE is released "to the wild" in FF42, adaptive streaming player's, like bitmovin's bitdash player (http://www.dash-player.com/), the HTML5 playback will be used. But not having cuechange events is kind of a blocker for us as we use cues for subtitles as well for other information. As you can see at http://www.dash-player.com/demo/multi-language-and-multi-audio/ in FF42 (with MediaSource enabled for everyone), no subtitles are shown due to cuechange event not firing.

Comment 30

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

It definitely won't get fixed in Firefox 42. We don't have any work on text tracks scheduled at this point in time.

Comment 31

[Mardeg]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #30)
> It definitely won't get fixed in Firefox 42. We don't have any work on text
> tracks scheduled at this point in time.

That's a shame, using capability from this and/or bug 996333 / bug 865395 would be the native API way to manually workaround bug 992664.

Comment 32

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Ralph - any thoughts?

Comment 33

[Ralph Giles (:rillian)]

I agree it's a shame. These remaining event bugs aren't too hard, but we need someone to work on them.

Comment 34

[ruxton]

Attachment: Bug-1033144---Part-1-Dispatch-cuechange-event-on-T.patch

Aligns reyre's Part 1 with current development

Comment 35

[ruxton]

Attachment: Bug-1033144---Part-2-Add-tests-for-TextTrack-cuech.patch

Aligns reyre's previous work with current development and updates the test slightly.

Comment 36

[ruxton]

Ok so I've gone and aligned the patches work with the current master on github.  I've also adjusted the test slightly to individually test they're firing as 6, rather than as a group of 12 and only pass upon end. 

I feel like there should be some kind of "timing out" so if the ended event doesn't fire off, the test will fail.  I was just going to base that on the VTT's length, but that makes the test brittle, so I avoided that.

I have some another test and commit that hang off this work to make the cuechange fire for audio tags properly too.  Right now, this only makes video tags work.

Comment 37

[ruxton]

Attachment: 0001-Bug-1033144-Part-1-Dispatch-cuechange-event-on-TextT.patch

accidentally uploaded the wrong patch.

Comment 38

[ruxton]

Attachment: 0002-Bug-1033144-Part-2-Add-tests-for-TextTrack-cuechange.patch

uploaded the wrong patch.

Comment 39

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Ralph - are you following this?

Comment 40

[Ralph Giles (:rillian)]

Thanks for the patched, ruxton. Sorry for not responding yesterday. Let me try them out.

Comment 41

[Ralph Giles (:rillian)]

Comment on attachment 8714165 [details] [diff] [review]
0002-Bug-1033144-Part-2-Add-tests-for-TextTrack-cuechange.patch

Review of attachment 8714165 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the update. Looks resonable to move this forward r=me with comments addressed.

(That means update the patch or explain why review suggestions are wrong. :)

Not having a timeout is unfortunate. You do have a listener on 'ended', so it should terminate as long as playback completes, which is probably good enough. The test harness itself has a timeout which will trigger eventually, but it's minutes away and relying on it slows down the test run. This file is seekable, so after metadataloaded is available, video.duration should be available, so you could set a timeout based on that, but you have to worry about it racing with the successful end of the test.

Dan's been doing some work to clean up test flakiness lately. Dan, do you have any commets on this patch?

::: dom/media/test/test_texttrackevents_video.html
@@ +14,5 @@
> +<pre id="test">
> +<script class="testbody" type="text/javascript">
> +SimpleTest.waitForExplicitFinish();
> +SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true],
> +                                   ["media.webvtt.regions.enabled", true]]},

This test doesn't use regions, which we're unlikely to implement anyway. No reason to keep propagating the preference for it. So just:

  SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]},

should work fine.

@@ +31,5 @@
> +    var trackElementCueChangeCount = 0;
> +    var trackCueChangeCount = 0;
> +
> +    video.addEventListener("loadedmetadata", function run_tests() {
> +      // Re-que run_tests() at the end of the event loop until the track

// Re-queue

@@ +48,5 @@
> +        ++trackCueChangeCount;
> +      }
> +
> +      trackElement.track.oncuechange = countTrackCueChange;
> +      trackElement.addEventListener("cuechange", countCueChange);

FWIW, anonymous functions would be fine here. Up to you though.

@@ +57,5 @@
> +    video.addEventListener('ended', function() {
> +      // Should be fired 6 times, as there are 3 cues, with a change event
> +      // for when it is activated/deactivated (6 events total)
> +      ok((trackElementCueChangeCount === 6),"TrackElement should fire cue change 6 times.");
> +      ok(trackCueChangeCount === 6,"TrackElement.track should fire cue change 6 times.");

Mochitest has an is() method which is easier to read than the boolean. e.g.

  is(trackCueChangeCount, 6, "TrackElement.track should fire cue change 6 times.");

See https://developer.mozilla.org/en-US/docs/Mochitest#Test_functions for a full list. It also now claims to use `===` contrary to my comment #11. But `==` works for integers, I think.

Either way, please put a space after the comma separating arguments.

Comment 42

[u480271]

Comment on attachment 8714165 [details] [diff] [review]
0002-Bug-1033144-Part-2-Add-tests-for-TextTrack-cuechange.patch

Review of attachment 8714165 [details] [diff] [review]:
-----------------------------------------------------------------

Ralph, I've been looking at test flakiness caused by my own changes to C++ code.

LGTM.

Comment 43

[Ralph Giles (:rillian)]

Thanks for taking a look.

Comment 44

[Ralph Giles (:rillian)]

Comment on attachment 8714164 [details] [diff] [review]
0001-Bug-1033144-Part-1-Dispatch-cuechange-event-on-TextT.patch

Review of attachment 8714164 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok to me. bz, you said you wanted to check the async dispatch again.

Comment 45

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8714164 [details] [diff] [review]
0001-Bug-1033144-Part-1-Dispatch-cuechange-event-on-TextT.patch

r=me

Comment 46

[Ralph Giles (:rillian)]

Need to un-exfail the corresponding web platform test.

TEST-UNEXPECTED-PASS | /html/dom/interfaces.html | TextTrack interface: attribute oncuechange - expected FAIL

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8521df7b7e3c&selectedJob=16454881

Comment 47

[ruxton]

Attachment: 0002-Bug-1033144-Part-2-Add-tests-for-TextTrack-cuechange.patch

Comment 48

[ruxton]

Attachment: 0003-1033144-Part-3-Update-web-platform-tests-to-expect-P.patch

Attachment updates the web-platform-tests to expect PASS for oncuechange on TextTrack r=rillian

Comment 49

[Ralph Giles (:rillian)]

Pushed the new patches to try https://treeherder.mozilla.org/#/jobs?repo=try&revision=d09dedb8b440

Comment 50

[Ralph Giles (:rillian)]

Comment on attachment 8719644 [details] [diff] [review]
0003-1033144-Part-3-Update-web-platform-tests-to-expect-P.patch

Review of attachment 8719644 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, but let's wait for confirmation from the try server builds.

Comment 51

[Ralph Giles (:rillian)]

Comment on attachment 8719630 [details] [diff] [review]
0002-Bug-1033144-Part-2-Add-tests-for-TextTrack-cuechange.patch

Review of attachment 8719630 [details] [diff] [review]:
-----------------------------------------------------------------

r=me Thanks for following up, and sorry for the long delay.

I worry having waitForExplicitFinish() outside the pushPrefEnv might cause problems on e10s, but we have several other tests like that, so we can fix it separately if it's an issue.

Comment 52

[Ralph Giles (:rillian)]

Green on try. I'll fix up the commit messages to conform to style and land. Thanks for contributing!

NB if you do more patches in the future, setting an explicity r? flag on the the attachment is a good way to make sure it gets attention in bugzilla. We don't track r= or r? in comments of the commit message like github does.

Comment 53

[Ralph Giles (:rillian)]

Attachment: Part 3: Update web platform tests

Rebase over yesterday's additions which moved the changes outside the fuzz window. Carrying r=me.

Comment 54

[Ralph Giles (:rillian)]

I've pushed your changes to the inbound queue. They'll go through a full set of tests there and if all goes well, will be merged to the central repo later today and appear in a subsequent nightly build.

Comment 55

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8935a32269a5
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1d06acdeb7b
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f4d815bb9af

Comment 56

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8935a32269a5
https://hg.mozilla.org/mozilla-central/rev/e1d06acdeb7b
https://hg.mozilla.org/mozilla-central/rev/9f4d815bb9af

Comment 57

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Ralph, could you please fill out the relnote template with suggested wording? That would be a lot of help. Thanks!

Comment 58

[Ralph Giles (:rillian)]

Sorry, missed the template when I set the flag.

Release Note Request (optional, but appreciated)
[Why is this notable]: This feature is helpful to authors writing media players, and has been a deficiency of our webvtt support.
[Suggested wording]: Developer: `cuechange` events are now available on TextTrack objects.
[Links (documentation, blog post, etc)]: https://html.spec.whatwg.org/multipage/embedded-content.html#event-media-cuechange

Comment 59

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks Ralph! Added to Fx47 (Aurora) release notes.