Authentication-Results: mail-b.sr.ht; dkim=pass header.d=disroot.org header.i=@disroot.org Received: from knopi.disroot.org (knopi.disroot.org [178.21.23.139]) by mail-b.sr.ht (Postfix) with ESMTPS id 7417611EF1F for <~cadence/tube-devel@lists.sr.ht>; Sun, 14 Mar 2021 13:10:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id 7B8A050EB5 for <~cadence/tube-devel@lists.sr.ht>; Sun, 14 Mar 2021 14:10:56 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at disroot.org Received: from knopi.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ft2Mx5w6Tda8 for <~cadence/tube-devel@lists.sr.ht>; Sun, 14 Mar 2021 14:10:55 +0100 (CET) Date: Sun, 14 Mar 2021 13:10:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1615727454; bh=MGjWxRJ9G1L2vYy0fIhaXrQKVN8UxNOl6LJjCAdhu44=; h=Date:From:To:In-Reply-To:References:Subject; b=izTDzKlSf+aHlclXXqmbrW98ATvsM2CyqGiATahBbCIRa/WRjQ/Rf0kYsXMYLSE/D zKjLVhjlyhTrOK6+ka0KxgTnOjxIH6wp4RF7mIkBR7tiHSERU57Ll+zYyrTWENnnTI xsXFOPvVK62ogREZZgN6zyixwc83WPZ7t6D9zIghGfNsWqp/p+Jc9kmFY+zpqP8FvM BVGxhz7OOXP1yHlPaJrCJVfohGITZrmzlZLD5OX3CpfREd2CoCYpLIbBxkw4l/jkoa ep2bcG2FbluRg7A6wYiQMHWmj2m9YewJ/cqfAK2BN72v8uWPC7ceB56OQ3G+cuncpF QA02KRRR/V1Qw== From: Cadence Ember To: ~cadence/tube-devel@lists.sr.ht Message-ID: In-Reply-To: <20210314064924.21503-1-sir@hacktivista.com> References: <20210314064924.21503-1-sir@hacktivista.com> Subject: [PATCH] Captions support Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Correlation-ID: Thanks so much for this! Tomorrow I can properly review it, but taking a peek at the code now, there's a couple of things that stand out to me: - kind should be `captions` since it is a transcription of audio, not a translation; see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/track - should put the result of `getSettingsOrDefaults` in a variable to make it easier to extract other settings in the future - should validate the `lang` query parameter somehow I can deal with these notes on my own tomorrow, as well as doing my full review, but if you want make these changes yourself, you're free to submit an updated patch.