✨ Add volume attribute to amp-video for use in amp-story-page (#36693)

* Add volume support in amp-video for amp-story. * Parse volume string to float. * Update regex for volume. * Update extensions/amp-story/1.0/media-pool.js Co-authored-by: Gabriel Majoulet <gmajoulet@google.com> * Add validator test cases. Co-authored-by: Gabriel Majoulet <gmajoulet@google.com>
ampproject · Nov 2, 2021 · 8aa4abe · 8aa4abe
1 parent bdc9729
commit 8aa4abe
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 1 deletion.
diff --git a/extensions/amp-story/1.0/media-pool.js b/extensions/amp-story/1.0/media-pool.js
@@ -850,6 +850,11 @@ export class MediaPool {
       return Promise.resolve();
     }
 
+    // When a video is muted, reset its volume to the default value of 1.
+    if (mediaType == MediaType.VIDEO) {
+      domMediaEl.volume = 1;
+    }
+
     return this.enqueueMediaElementTask_(poolMediaEl, new MuteTask());
   }
 
@@ -870,6 +875,16 @@ export class MediaPool {
       return Promise.resolve();
     }
 
+    if (mediaType == MediaType.VIDEO) {
+      const ampVideoEl = domMediaEl.parentElement;
+      if (ampVideoEl) {
+        const volume = ampVideoEl.getAttribute('volume');
+        if (volume) {
+          domMediaEl.volume = parseFloat(volume);
+        }
+      }
+    }
+
     return this.enqueueMediaElementTask_(poolMediaEl, new UnmuteTask());
   }
 

diff --git a/extensions/amp-story/1.0/test/validator-amp-story-video-error.html b/extensions/amp-story/1.0/test/validator-amp-story-video-error.html
@@ -32,6 +32,40 @@
          </amp-video>
       </amp-story-grid-layer>
     </amp-story-page>
+    <amp-story-page id="2" background-audio="path/to/my.mp3" auto-advance-after="any-value">
+      <amp-story-grid-layer template="horizontal">
+        <!-- This should fail because amp-video volume should be within [0.1, 1]. -->
+        <amp-video width="480"
+                   height="270"
+                   poster="/foo.jpg"
+                   src="/video/tokyo.mp4"
+                   layout="responsive"
+                   volume="0">
+             <div fallback>
+               <p>Your browser doesn't support HTML5 video.</p>
+             </div>
+             <source type="video/mp4" src="/video/tokyo.mp4">
+             <source type="video/webm" src="/video/tokyo.webm">
+         </amp-video>
+      </amp-story-grid-layer>
+    </amp-story-page>
+    <amp-story-page id="3" background-audio="path/to/my.mp3" auto-advance-after="any-value">
+      <amp-story-grid-layer template="horizontal">
+        <!-- This should fail because amp-video volume should be within [0.1, 1]. -->
+        <amp-video width="480"
+                   height="270"
+                   poster="/foo.jpg"
+                   src="/video/tokyo.mp4"
+                   layout="responsive"
+                   volume="1.01">
+             <div fallback>
+               <p>Your browser doesn't support HTML5 video.</p>
+             </div>
+             <source type="video/mp4" src="/video/tokyo.mp4">
+             <source type="video/webm" src="/video/tokyo.webm">
+         </amp-video>
+      </amp-story-grid-layer>
+    </amp-story-page>
   </amp-story>
 </body>
 </html>
diff --git a/extensions/amp-story/1.0/test/validator-amp-story-video-error.out b/extensions/amp-story/1.0/test/validator-amp-story-video-error.out
@@ -37,6 +37,44 @@ amp-story/1.0/test/validator-amp-story-video-error.html:23:8 The mandatory attri
 |           </amp-video>
 |        </amp-story-grid-layer>
 |      </amp-story-page>
+|      <amp-story-page id="2" background-audio="path/to/my.mp3" auto-advance-after="any-value">
+|        <amp-story-grid-layer template="horizontal">
+|          <!-- This should fail because amp-video volume should be within [0.1, 1]. -->
+|          <amp-video width="480"
+>>         ^~~~~~~~~
+amp-story/1.0/test/validator-amp-story-video-error.html:38:8 The attribute 'volume' in tag 'amp-video' is set to the invalid value '0'. (see https://amp.dev/documentation/components/amp-video/)
+|                     height="270"
+|                     poster="/foo.jpg"
+|                     src="/video/tokyo.mp4"
+|                     layout="responsive"
+|                     volume="0">
+|               <div fallback>
+|                 <p>Your browser doesn't support HTML5 video.</p>
+|               </div>
+|               <source type="video/mp4" src="/video/tokyo.mp4">
+|               <source type="video/webm" src="/video/tokyo.webm">
+|           </amp-video>
+|        </amp-story-grid-layer>
+|      </amp-story-page>
+|      <amp-story-page id="3" background-audio="path/to/my.mp3" auto-advance-after="any-value">
+|        <amp-story-grid-layer template="horizontal">
+|          <!-- This should fail because amp-video volume should be within [0.1, 1]. -->
+|          <amp-video width="480"
+>>         ^~~~~~~~~
+amp-story/1.0/test/validator-amp-story-video-error.html:55:8 The attribute 'volume' in tag 'amp-video' is set to the invalid value '1.01'. (see https://amp.dev/documentation/components/amp-video/)
+|                     height="270"
+|                     poster="/foo.jpg"
+|                     src="/video/tokyo.mp4"
+|                     layout="responsive"
+|                     volume="1.01">
+|               <div fallback>
+|                 <p>Your browser doesn't support HTML5 video.</p>
+|               </div>
+|               <source type="video/mp4" src="/video/tokyo.mp4">
+|               <source type="video/webm" src="/video/tokyo.webm">
+|           </amp-video>
+|        </amp-story-grid-layer>
+|      </amp-story-page>
 |    </amp-story>
 |  </body>
-|  </html>
+|  </html>
diff --git a/extensions/amp-video/validator-amp-video.protoascii b/extensions/amp-video/validator-amp-video.protoascii
@@ -183,6 +183,12 @@ tags: {  # <amp-video> in amp-story
     name: "captions-id"
     requires_extension: "amp-story-captions"
   }
+  attrs: {
+    name: "volume"
+    # Disallow 0 to prevent shipping unnecessary audio track when the video is muted.
+    # Volume must be in the range [0.1, 1].
+    value_regex: "^((0?\.[1-9]+)?|1(\.0*)?)$"
+  }
   attr_lists: "extended-amp-global"
   attr_lists: "amp-video-common"
   spec_url: "https://amp.dev/documentation/components/amp-video/"