cloudinary icon indicating copy to clipboard operation
cloudinary copied to clipboard

rawTransformations needed on <CldVideoPlayer /> like on useCldVideoUrl

Open billnbell3 opened this issue 1 year ago • 15 comments

It would be awesome if we can send rawTransformations in CldVideoPlayer like we can in useCldVideoUrl!

The transformations property just appends t_ and does not do what we want.

See below for how we got it to work without CldVideoPlayer:

<template>
  <video
    ref="videoPlayerRef"
    v-if="cldVideo && isCldVideo"
    :src="getVideoUrl()"
    :loop="isCldVideoLoop"
    autoPlay="always"
    :controls="isCldVideoSound"
    :quality="quality"
    :muted="!isCldVideoSound"
  />
  <!-- <CldVideoPlayer
    ref="videoPlayerRef"
    v-if="cldVideo && isCldVideo"
    :src="src2"
    :loop="isCldVideoLoop"
    autoPlay="always"
    :playsinline="true"
    :controls="isCldVideoSound"
    :muted="!isCldVideoSound"
    :quality="quality"
    :fetch-format="format"
    controlsList="nodownload"
    :config="{ autoplay: true }"
  /> -->
</template>

<script>
  export default {
    props: {
      //cloudinary video props
      cldVideo: {
        type: Object,
        default: null,
      },
      isCldVideoQAuto: {
        type: Boolean,
        default: true,
      },
      isCldVideoFAuto: {
        type: Boolean,
        default: true,
      },
      isCldVideoSound: {
        type: Boolean,
        default: false,
      },
      isCldVideoLoop: {
        type: Boolean,
        default: true,
      },
      isCldVideoFadeEffect: {
        type: Boolean,
        default: false,
      },
      isCldVideo: {
        type: Boolean,
        default: false,
      },
    },
    data() {
      return {
        quality: String,
        format: String,
        videoId: String,
      };
    },
    created() {
      //mapping width and height from selected dimensions option
      if (this.isCldVideo) {
        this.quality = this.isCldVideoQAuto ? 'auto' : '';
        this.format = this.isCldVideoFAuto ? 'auto' : '';
        this.videoId = this.cldVideo?.public_id.replace(/ /g, '%20');
      }
    },
    methods: {
      getVideoUrl() {
        const { url } = useCldVideoUrl({
          options: {
            src: this.videoId,
            quality: this.quality,
            controls: this.isCldVideoSound,
            autoplay: true,
            controlsList: 'nodownload',
            fetchFormat: this.format,
            rawTransformations: this.isCldVideoFadeEffect
              ? 'e_fade:2000/e_fade:-2000'
              : '',
          },
        });
        return url;
      },
    },
  };
</script>
<style>
  .video-js.vjs-fluid:not(.vjs-audio-only-mode) {
    min-height: 100%;
  }
  .cld-video-player.cld-fluid {
    height: 100%;
  }
  .video-js .vjs-tech {
    object-fit: cover;
  }
</style>

billnbell3 avatar Sep 25 '24 01:09 billnbell3

Hey there!

Thanks for reporting this issue. I do agree that it would be useful to have it on the component level. Would you be interested in contributing to the module with this change?

I can provide all the help needed to make it live :)

Baroshem avatar Sep 25 '24 03:09 Baroshem

hey wanted to chime in here because part of my goal with Next.js, the cousin to this library, is to have a similar API To the CldImage component as far as transformations go

I wasn't sure if the underlaying options for the Cloudinary Video Player supported this capability but I found that it appears to:

https://github.com/cloudinary/js-url-gen/blob/master/src/types/types.ts#L433

so technically speaking, you could as of now, pass in transformation={{ raw_transformation: 'string' }} and it would work

image

however I think it should be exposed as a top level option similar to CldImage from that consistency POV

to allow this library, Next, Svelte, and Astro to all benefit from this together, I would love to see this change added to our core library, Cloudinary Util: https://github.com/colbyfayock/cloudinary-util

whether one of you handle it, i handle it, or perhaps someone handles it through Hacktoberfest, I set up a new issue

https://github.com/cloudinary-community/cloudinary-util/issues/201

colbyfayock avatar Sep 25 '24 13:09 colbyfayock

@billnbell3 aside from this feature support - i would be curious about what your use case is for needing raw transformations and if there isn't a specific transformation that we need to add support for?

colbyfayock avatar Sep 25 '24 13:09 colbyfayock

Yeah fading in and out is pretty common for videos.

billnbell3 avatar Sep 25 '24 18:09 billnbell3

Also your screenshot is not Vue code.?

Generally we need to wrap the object is double quotes in Vuejs.

transformation="{ raw_tranformation: 'e_fade:2000'}"

Does that work for you? It does not for me.

billnbell3 avatar Sep 25 '24 18:09 billnbell3

yeah sorry for the confusion, i was testing in the nextjs equivalent: https://next.cloudinary.dev/cldvideoplayer/basic-usage

colbyfayock avatar Sep 25 '24 18:09 colbyfayock

It actually does not work in VueJs. Probably needs encoding fix or something.

This is what I tried - no fading is in the URL when using this.

<CldVideoPlayer
    ref="videoPlayerRef"
    :src="videoId"
    autoPlay="always"
    :playsinline="true"
    :quality="quality"
    :fetch-format="format"
    controlsList="nodownload"
    :config="{ autoplay: true }"
    :transformation="{ transformation_raw: 'e_fade:2000,e_fade:-2000' }"
  /> 

billnbell3 avatar Sep 25 '24 20:09 billnbell3

can you try

    :transformation="{ raw_transformation: 'e_fade:2000,e_fade:-2000' }"

had the naming backwards

colbyfayock avatar Sep 26 '24 01:09 colbyfayock

yep that works. The docs need upgrading. And the raw_transformation should be matching to other libraries.

billnbell3 avatar Sep 26 '24 04:09 billnbell3

@colbyfayock do you think these raw_transormation should be added as a top level prop? Or just added to the docs that it can be used inside transformation prop?

Baroshem avatar Sep 26 '24 09:09 Baroshem

however I think it should be exposed as a top level option similar to CldImage from that consistency POV

yeah my goal is to make the API between the video player and image component consistent, so it woudl make sense as a top level prop. ensuring the player options are documented and typed are important for a stop-gap solution and also for the other options that are already available. i have a separate ticket open to update the types

https://github.com/cloudinary-community/cloudinary-util/issues/202

colbyfayock avatar Sep 26 '24 12:09 colbyfayock

Caveat: unless we decide to do a CldVideo component which I've considered in the past, the idea being it wraps the native <video element instead of bringing in the Video Player. Less customization and features, but if people dont care about that, they can ship less JS, so having that option could be beneficial. i still think we should make some APIs consistent, like rawTransformations as it makes sense, but the API for CldVideo could be almost identical to CldImage

colbyfayock avatar Sep 26 '24 12:09 colbyfayock

I see thanks for the comment @colbyfayock and sorry for long response. This comment of your completely disapeard from my notifications.

I would be up for adding this raw Transformations to the prop dearation of the component to support the same as useCldVideoUrl for now while in the background we could think about building CldVideo.vue component.

Would you like me to create an RFC based on this issue? :)

@billnbell3 would you be up for contributing to the module and adding these rawtransformations to CldVideoPlayer? I can provide all help needed :)

Baroshem avatar Oct 11 '24 05:10 Baroshem

if you think the CldVideo component would be valuable let's do it!

colbyfayock avatar Oct 11 '24 18:10 colbyfayock

I have no time to do this right now.

billnbell3 avatar Oct 11 '24 18:10 billnbell3

@colbyfayock

I have added a note about raw transformations to CldVideoPlayer docs. For the thing about CldVideo component, I think first we would need to have an RFC about this component, gather feedback about how it should look like and behave. I will talk about it with Sanjay :)

Baroshem avatar Nov 05 '24 08:11 Baroshem