1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

http://www.artima.com/cppsource/nevercall.html

ResetDecode() is a virtual method and it is bad to call it in the destructor. Furthermore, I don't see the necessity to call it in the destructor.

nsresult MediaDecoderReader::ResetDecode()
{
  VideoQueue().Reset(); <-- we don't care about it since it will be destroyed soon.
  AudioQueue().Reset(); <-- ditto.

  mAudioDiscontinuity = true; <-- don't care
  mVideoDiscontinuity = true; <-- don't care

  mBaseAudioPromise.RejectIfExists(CANCELED, __func__); <-- already rejected in Shutdown()
  mBaseVideoPromise.RejectIfExists(CANCELED, __func__); <-- ditto.

  return NS_OK;
}

We should be fine without calling ResetDecode() in the destructor.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: MozReview Request: Bug 1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader. r=gerald.

Bug 1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader. r=gerald.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8675489 [details]
MozReview Request: Bug 1215439 - Don't call ResetDecode() in the destructor of MediaDecoderReader. r=gerald.

https://reviewboard.mozilla.org/r/22381/#review19975

Makes sense.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the review!

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8aa35040b3bf

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8aa35040b3bf